Design choice: Do I want extension methods to throw exceptions on null? [duplicate]

Question

Possible Duplicate:
C#: Best practice for validating “this” argument in extension methods

I'm ambivalent about a design choice, and would like to hear the opinions of the SO community. The example I bring up here is just one possible case where this design choice has to be made - in reality, there might be many more cases. Answers are welcome both to this specific case and to a more general approach, and guidelines on how to make the decision in a specific case are also appreciated.

Basically, I want to know how to think about this: When writing an extension method that doesn't intrinsically fail if a null reference is passed as the this instance, should a null check be performed on the argument or not?

Example:

I'm writing an extension method on IEnumerable<T> that will iterate through the collection and performe some Action<T> - basically, this is what it'll do:

public static void Each<T>(this IEnumerable<T> collection, Action<T> action)
{
    foreach (var t in collection)
    {
        action.Invoke(t);
    }
}

What I cannot decide about, is what this extension method should do if null is passed into either parameter. If I do not add any null checks, I will get a NullReferenceException on action.Invoke(T), but if collection is null the for loop will just silently do nothing (and no exception will be thrown even if action is also null...).

I am quite decided to add a null check for action, so I can throw an ArgumentNullException instead of the NullReferenceException. But what do I want to do about the collection?

Option 1: Add a null check, and throw ArgumentNullException.

Option 2: Just silently let the method do nothing.

Which will be more useful where I might want to use the method in the future? Why?

Answer 1

Microsoft throws an ArgumentNullException if the collections invoked in LINQ are empty. This is really more a matter of style, but is consistent with the way extension methods are supposed to behave.

@m0sa is right though in that you'll get a null reference from your foreach, but I would say check up top and throw ArgumentNullException. That way you'll be on par with what LINQ does.

For example, if you look at Any() in a decompiler you see:

public static bool Any<TSource>(this IEnumerable<TSource> source)
{
    if (source == null)
    {
        throw Error.ArgumentNull("source");
    }
    using (IEnumerator<TSource> enumerator = source.GetEnumerator())
    {
        if (enumerator.MoveNext())
        {
            return true;
        }
    }
    return false;
}

Answer 2

You should definitely do null checking and throw ArgumentNullException to avoid hard to understand NullReferenceExceptions inside your code.

In general I would avoid to treat null as en "empty" IEnumerable<T>. Just use Enumerable.Empty<T>() instead to avoid special cases for a null collection.

If you decide to do null checking in your extension method you should consider doing that "eagerly". If you are using yield return inside your extension method none of it will actually be evaluated until iteration begins. You split your extension method into two parts. The "eager" part where arguments are checked and the "lazy" that yield return elements.