September 2, 2008

The hazards of extension methods

I love extension methods! I really do, to the point that I have to constrain myself so that this blog won't turn into a shrine where followers of extension methods hang around all day praising the founders for giving us this precious gift.

That being said though I have one major issue with extension methods. Since exception methods is essentially glorified static methods they will accept you calling them even if the object reference is null.

string s = null;
s.ToString(); // This will throw a NullReferenceException
s.HtmlEncode(); // This will not

Now, did you see the difference? Since it's the string object we're talking about we all know that the HtmlEncode method doesn't belong there and thus we can deduce that it's an extension method but what if it where some lesser known object and the method name wasn't so obvious? It works since the s.HtmlEncode call will be compiled into something like this: StringExtensions.HtmlEncode(s)

Well, I thought about it for a while and I decided that I don't like it! I don't like it one bit actually and while I do respect the decision to make it this way I feel that it will essentially undermine the respect that C# developers have for null references. Whenever I look at some code calling an instance method there is something, embedded deep in my cerebral cortex, that tells me that I should watch out for nullity but when I see extension methods that allow (or actually depends on) the reference to be null my fear of NullReferenceException gradually goes away.

So what's my recommended solution? Well, with great power comes great responsibility and of course it's up to you whether or not you're going to include these methods in your code. I will not! Whenever I write an extension method I explicitly check for nullity and raise the ArgumentNullException.

The worst use of this this that I've seen so far is without a doubt the IsNull extension method which essentially lets you do this:

string s = null;

if(!s.IsNull())
    PerformWork(s)

if(s != null)
    PerformWork(s)

Now do you see the problem here? It's actually more code, less obvious, (IMO) less readable than the "original" null comparison and it breaks a very important rule; you can't call instance methods on null references. Now, I have no problem with the IsEmpty extension method since that doesn't break any null rules.

PS. This whole post is actually a reaction to the anonymous comment suggesting that I convert my Collection.IsNullOrEmpty method into an extension method. DS.

Licensing information

kick it on DotNetKicks.com

4 comments:

  1. You could just make sure you always initialize values.

    One of the firs things they tought me in school when learning C, is to always initalize values when you declare them. As such, over the years, I just always make sure I don't use null values.

    I find it makes things way easier.

    ReplyDelete
  2. Yes that is a good practice indeed but it's far from certain that your method is the one that declares the variable. It could be passed as a parameter or it could be defined in a class (which could lead to it getting redefined between or during method calls in a multithreaded environment). My point is that (and I believe that it's the same point that you're trying to make) you should have great respect for possible null references.

    ReplyDelete
  3. I had hoped that they would have made it more than a syntatic sugar. Perhaps how it works in Objective-C, and similar dynamic languages.

    They could have injected IL-code before the calls to trap null references. But perhaps that is not worth it, performance wise. Or LINQ perhaps needs those nulls.

    But I would prefer it to have the same call semantics as ordinary instance methdos.

    ReplyDelete
  4. No-one stops you from putting a normal guard method in your extension methods, so that they will throw ArgumentNullException if null is passed as a value.

    IIRC, most of the BCL extension methods do that...

    ReplyDelete