Wednesday, June 23, 2010

Extension method madness

by luisabreu via LA.NET [EN] on 6/23/2010 10:38:27 AM

Even though I haven’t been as active as I wished, I have still managed to get some time to read several interesting blog posts lately. Unfortunately, it seems like two guys that I have a lot of respect for (though I haven’t had the pleasure of meeting them in person) have gone into the “dark side”. I’m referring to Phil Haack and Oren Eini (aka Ayende). The problem: using an extension method for checking for an empty enumeration.

It all started with Phil’s post, where he presented an extension method for testing for an empty IEnumerable. Here’s the code he ended up with:

public static bool IsNullOrEmpty<T>(this IEnumerable<T> items) {
    return items == null || !items.Any();
}

Phil really explains why using the Any is a good option (especially when compared to Count) and I do agree with that part. Now, the problem here is that Ayende picked it up and improved it so that you don’t end up “loosing” items when you can’t go several times over an IEnumerable. Unfortunately, both of them missed the null check. Don’t understand what I mean? In that case, do take some time and look really carefully at Phil’s initial code. Do you see anything wrong? No? Let me ask you a question: what’s the expected result of the following code:

IEnumerable<Int32> nullCll = null;
var isEmpty = nullCll.IsNullOrEmpty();

In my opinion, it should throw a null reference exception. But that won’t happen in this case. Don’t you find that weird? I do and I think extension methods should behave the same way instance methods. And that really means null exceptions shouldn’t be “replaced” with Boolean values.

Btw, and that’s just my opinion, the helper method should really be a simple helper method (ie, not an extension method). Just because we have extension methods, it doesn’t meant they’re the solution to all the extension problems we have…