When should extension methods be avoided?

Question

Before you start pointing me to duplicates just know that I have read nearly all the posts on SO about extension methods. I am just trying to play devil's advocate for a minute to consider the alternative to my working opinion.

Recently I was working on a project and a need came up for a method to be a base of an interface. So I suggested we write an extension method and it was shot down. Saying it added complexity and harder to debug.

I of course argued and got on SO to find all the wonderful posts that show the many reasons why to use extension methods. Not to forget that a lot of the .net framework uses them. We eventually did not use it as I was overruled by the team.

But then it got me thinking, are there times when an extension method could be used but shouldn't be?

I really couldn't think of any but thought I would post here and see if anyone could think of any other reasons why they shouldn't be used.

Answer 1

Any time you have a function which is "generally applicable" to an object of a certain type, regardless of its state, an extension method is a good choice.

For example, today I added two new extension methods to our codebase:

public static XElement ToXElement(this XmlElement element) { }

public static XmlElement ToXmlElement(this XElement element) { }

Both of these are, generally speaking, valid on the types they extend regardless of the state of the instance or where we are using it.

If your method does not meet that criteria, it should probably be moved to a helper method closer to the context where the particular case is always true or easily checked.

For example, a developer recently nominated this to be an extension method:

public static bool ParseYesNoBool(this string input) { }

There are two problems here: first, this will appear on all strings in the application, even though the number of strings which might ever be candidates for this case are very small. So we've broken the first rule, in that it is not useful regardless of state. Similarly, but second, the consumer of this functionality is limited to a single parser for one particular connector to an external system. So promoting implementation-specific functionality into the general-use namespace makes no sense. This was downgraded to a helper method in the parser.

As far as readability and debugging, that is just incorrect for a developer of any reasonable skill level.

Answer 2

In general if you control the source-code for the assembly and adding the method does not cause any breaking changes to existing code (which would have been the case if for example LINQ had not been implemented via extension methods) it is better to just add a normal method.

Answer 3

This discussion of the Framework Design Guildelines section on extension methods contains some good advice. I think the relevant portion for your scenario is:

To provide helper functionality relevant to every implementation of an interface, if said functionality can be written in terms of the core interface.

If your proposed usage did not pass that test then it should have been shot down.