views:

540

answers:

7

We are currently discussing whether Extension methods in .NET are bad or not. Or under what circumstances Extension methods can introduce hard to find bugs or in any other way behave unexpectedly.

We came up with:

  • Writing an extension method for types that are not under your control (e.g. extending DirectoryInfo with GetTotalSize(), etc...) is bad, because the owner of the API could introduce a method that hides our extension - and might have different edge cases. For example testing for null in an extension method will automatically translate into a NullReferenceException if the extension method is no longer used due to hiding.

Question:

  • Are there any other dangerous situations than "hiding" that we are not thinking of?

Edit:

Another very dangerous situation. Suppose you have an extension method:

namespace Example.ExtensionMethods
{
    public static class Extension
    {
        public static int Conflict(this TestMe obj)
        {
            return -1;
        }
    }
}

And use it:

namespace Example.ExtensionMethods.Conflict.Test
{
    [TestFixture]
    public class ConflictExtensionTest
    {
        [Test]
        public void ConflictTest()
        {
            TestMe me = new TestMe();
            int result = me.Conflict();
            Assert.That(result, Is.EqualTo(-1));
        }
    }
}

Notice that the namespace where you use it is longer.

Now you reference a dll with this:

namespace Example.ExtensionMethods.Conflict
{
    public static class ConflictExtension
    {
        public static int Conflict(this TestMe obj)
        {
            return 1;
        }
    }
}

And your Test will fail! It will compile without a compiler error. It will simply fail. Without you even having to specify "using Example.ExtensionMethods.Conflict". The compiler will walk the namespace name and find Example.ExtensionMethods.Conflict.ConflictExtension before Example.ExtensionMethods.Extension and will use that without ever complaining about ambiguous extension methods. Oh the horror!

+7  A: 

Some curiosities:

  • extension methods might be called on null instances; this might be confusing (but sometimes useful)
  • the "hiding" issue is a biggie if they have different intent
  • equally, you might get a different extension method with the same name from 2 different namespaces; if you only have one of the two namespaces, this could lead to inconsistent behaviour (depending on which)...
  • ...but if somebody adds a similar (same signature) extension method in a second namespace that your code uses, it will break at compile time (ambiguous)

(edit) And of course, there is the "Nullable<T>/new()" bomb (see here)...

Marc Gravell
+1  A: 

What .Net calls extension methods are also a limited form a MonkeyPatching (try to ignore the php rant in there).

That should give you some material for your discussion.

Joel Coehoorn
+3  A: 

One breakage we've just found in the MoreLINQ project: if you write a generic extension method, it's impossible to make sure it will work with all types. We've got a method with this signature:

public static IEnumerable<T> Concat<T>(this T head, IEnumerable<T> tail)

You can't use that with:

"foo".Concat(new [] { "tail" });

because of the string.Concat method...

Jon Skeet
Often it will actually be better to call the extension method statically
Casebash
+1  A: 

First of all I believe your wording is a bit misleading. I take it you're talking about "types" and not "objects".

Secondly, the big advantage of extension methods is that you can add features to type you don't control. If you control the type, why not just modify the type instead of relying on extension methods?

Brian Rasmussen
thanks for the object -> type geek grammar error.The advantages are well understood. We are looking for dangers.
Tobias Hertkorn
+4  A: 

I disagree, the whole point of extension methods is to add your members to black boxed classes. Like everything else there are pitfalls, you must be mindful in naming, implementation and understand the pecking order of the methods.

Chuck Conway
The argument could be made that static helper classes are just as useful in that particular scenario - and far less dangerous.
Tobias Hertkorn
@Tobias In the end that's what it compiles too, a static class. If an extension method and a instance method have the same name the compiler has to make a decision. There is an elegance to extension methods.
Chuck Conway
+2  A: 

I've used Ruby on Rails for almost as long as I've used C#. Ruby allows you to do something similar to the new extension methods. There are, of course, potential problems if someone named the method the same, but the advantages of being able to add methods to a closed class far outweigh the potential dissadvantage (which would probably be caused by bad design or poor planning).

Matt Grande
+1  A: 

One thing you can do to make sure extension methods don't conflict with other methods (extension or otherwise) is to use FxCop with rules such as the rule for checking for duplicate signatures.

Kit