ArgumentNullException or NullReferenceException from extension method?

Question

What would you consider to be the best exception type to throw when an extension method is called on a null instance (where the extension method does not allow it)? Since extension methods are nothing but static methods you could argue that it should be ArgumentNullException, but on the other hand they're used like instance methods so it might be more natural to use the NullReferenceException. Let's take the following example:

public static string ToInvariantString(this IFormattable value, string format)
{
    return value.ToString(format, CultureInfo.InvariantCulture);
}

This way a NullReferenceException will be thrown if the value parameter is null.

The other example would be:

public static string ToInvariantString(this IFormattable value, string format)
{
    if (value == null) throw new ArgumentNullException("value");
    return value.ToString(format, CultureInfo.InvariantCulture);
}

EDIT: In some of the answers you have pointed out that an extension methods can be called like a static method and in those cases a null reference exception would be wrong, which is a great point, and actually one of my concerns, not sure why I forgot to mention that in the question in the first place.

Someone also pointed out that it's wrong to throw a NullReferenceException, and yes, it is. That's why I don't throw it, I just let it happen (let the CLR throw it) by not guarding the method.

I think I favor the ArgumentNullException (that's what I've use so far) but I still think there is at least room to argue for an against the NullReferenceException since it seems more natural in most places where the method is going to be used.

Deventer

Answer 1

In general, exceptions included, you should treat an extension method as if it were a normal static method. In this case you should throw an ArgumentNullException.

Throwing a NullReferenceException here is a bad idea for a few reasons

• A null reference did not actually occur so seeing one is counterintuitive
• Throwing a NullReferenceException and causing a NullReferenceException to occur produce discernably different exceptions (One way to see the difference is the error code). This is true of many exceptions that are thrown by the CLR.

See When can you catch a StackOverflowException (a post I did on this subject).

• It's perfectly legal to call an extension method just as if it were a regular method. In that case I would certainly not except a NullReferenceException, but instead an ArgumentNullException.

Wittgenstein

Great comments and as I mention in my edit of the post this is the reason i don't explicitly throw the NullReferenceException, I still let the CLR throw it. – Epicene

"A null reference did not actually occur". Yes it did; you passed a null reference and the method tried to dereference it. – Ernesternesta

@Ernesternesta the method tried to dereference it, not the runtime! – Presbyterate

@OhadSchneider Actually, it also didn't. – Hayrack

A NullReferenceException actually indicates a null dereference. Tweak your terminology and point one stands. Ultimately, though, it really comes down to whether you want to treat extension methods as instance or static. If the best practice were to treat them as instance (which, afaik, it isn't) throwing a NullReferenceException would be the most intuitive, "technically" correct or not. – Jeanejeanelle

@JaredPar: It's also perfectly legal to assign a value through reflection to a read-only variable or to create an instance of a class without calling any constructors - it's just not what the creator of the API intended, therefore you have to do it at your own risk. – Circinate

Answer 2

Aside from all the other answers (which are good) I think it's worth looking at what Microsoft does for the sake of consistency... and the extension methods in Enumerable all throw ArgumentNullException as far as I can see.

Agley

Great idea, I had that myself, didn't do it, not sure why! ;-) – Epicene

This is a useful technique which I use myself very often. However, one should always bear in mind that just because the FCL uses a particular method to acheive something, it doesn't mean that method is optimal or correct. I recommend thinking of FCL methods as suggestions for how to achieve something, but the merit of a given method ought to be compared to alternatives before judging whether it the best choice. – Takeshi

Microsoft does both, that does not help. – Circinate

@Christoph: Could you give examples? My experience is that MS is pretty good about doing this well (using ArgumentNullException). – Agley

Answer 3

Since extension methods can be used in C# 2.0, and they can be called just like static methods (you do not HAVE to use them as extension methods), you should use ArgumentNullException.

Just because they look like methods on the type doesn't mean that they are, or are always called like one.

Jefferey

Answer 4

From the user's standpoint, the method looks and acts like an instance method, so if I were them, I would expect to see a NullReferenceException.

That said, I would suggest throwing either one or the other explicitly in the code, instead of just "happening" to throw one as in your first example.

Stillborn

Yes, I've thought of throwing the NullReferenceException explicitly to, but my feeling is that it should be reserved for the compiler really, but you may very well be right. – Epicene

From a users standpoint an extension method can be called both by extension or as a static method. – Wittgenstein

I agree with mqp and disagree with JaredPar: From a users standpoint an extension method should never be called through the static interface and it should always behave exactly as the instance method would. Therefore, I would recommend to throw a NullReferenceException (yourself) without any parameter name. Do not allow other error handling (of the first parameter) in your extension methods - although it's cool and often seen e.g. to return an empty string when calling a .Left(..) extension method on an uninitialized string, but totally unexpected and nontransparent. – Circinate

Answer 5

ArgumentNullException. There is no requirement to call extension methods as though they were instance methods. You can call them as if they were normal methods. NullReferenceException would be completely incorrect in that case.

Arty

Answer 6

To add to the confusion, Microsoft does both, throwing ArgumentNullExceptions as well as NullReferenceExceptions.

This example that throws an implicite NullReferenceException is from Roslyn (src\Workspaces\CSharp\Portable\Extensions\StringExtensions.cs):

internal static class StringExtensions
{
    public static string EscapeIdentifier(
        this string identifier,
        bool isQueryContext = false)
    {
        var nullIndex = identifier.IndexOf('\0');
        if (nullIndex >= 0)
        {
            identifier = identifier.Substring(0, nullIndex);
        }

        var needsEscaping = SyntaxFacts.GetKeywordKind(identifier) != SyntaxKind.None;

        // Check if we need to escape this contextual keyword
        needsEscaping = needsEscaping || (isQueryContext && SyntaxFacts.IsQueryContextualKeyword(SyntaxFacts.GetContextualKeywordKind(identifier)));

        return needsEscaping ? "@" + identifier : identifier;
    }

This extension method that throws an ArgumentNullException from the .NET Framework (System.Core/System/Linq/Enumerable.cs):

public static IEnumerable<TResult> Select<TSource, TResult>(this IEnumerable<TSource> source, Func<TSource, TResult> selector) {
            if (source == null) throw Error.ArgumentNull("source");
            if (selector == null) throw Error.ArgumentNull("selector");
            if (source is Iterator<TSource>) return ((Iterator<TSource>)source).Select(selector);
            if (source is TSource[]) return new WhereSelectArrayIterator<TSource, TResult>((TSource[])source, null, selector);
            if (source is List<TSource>) return new WhereSelectListIterator<TSource, TResult>((List<TSource>)source, null, selector);
            return new WhereSelectEnumerableIterator<TSource, TResult>(source, null, selector);


   }

As mentioned in a comment above, I would recommend to implement extension methods exactly as instance methods and therefore throw a NullReferenceException if the this-parameter is null. If somebody calls my extension methods inappropriately they are free to do so but also have to expect inappropriate behavior (a NullReferenceException instead of an ArgumentNullException). But if they call the method as it is intended to, they should also get the expected behavior: a through out consistent experience.

//Instance method
string foo = null;
foo.Trim();

and

//Extension method
string foo = null;
foo.Right(10); 

They look alike and they should behave alike and the programmer should never even need to know whether it is an instance or an extension method.

Circinate