What is the best way to extend null check?

Question

You all do this:

public void Proc(object parameter)
{
    if (parameter == null)
        throw new ArgumentNullException("parameter");

    // Main code.
}

Jon Skeet once mentioned that he sometimes uses the extension to do this check so you can do just:

parameter.ThrowIfNull("parameter");

So I come of with two implementations of this extension and I don't know which one is the best.

First:

internal static void ThrowIfNull<T>(this T o, string paramName) where T : class
{
    if (o == null)
        throw new ArgumentNullException(paramName);
}

Second:

internal static void ThrowIfNull(this object o, string paramName)
{
    if (o == null)
        throw new ArgumentNullException(paramName);
}

What do you think?

Answer 1

I tend to stick to the ubiquitous Guard class for this:

static class Guard
{
    public static void AgainstNulls(object parameter, string name = null)
    {
        if (parameter == null) 
            throw new ArgumentNullException(name ?? "guarded argument was null");

        Contract.EndContractBlock(); // If you use Code Contracts.
    }
}

Guard.AgainstNulls(parameter, "parameter");

And shy away from extending object, plus to the naked eye a method call on a null object seems nonsensical (although I know it is perfectly valid to have null method calls against extension methods).

As for which is best, I'd use neither. They both have infinite recursion. I'd also not bother guarding the message parameter, make it optionally null. Your first solution will also not support Nullable<T> types as the class constraint blocks it.

Our Guard class also has the Contract.EndContractBlock() call after it for when we decide to enable Code Contracts, as it fits the "if-then-throw" structure that is required.

This is also a perfect candidate for a PostSharp aspect.

Answer 2

What about using Expression Trees (from Visual Studio Magazine):

using System;
using System.Linq.Expressions;
namespace Validation
{
   public static class Validator
   {
     public static void ThrowIfNull(Expression<Func<object>> expression)
     {
       var body = expression.Body as MemberExpression;
       if( body == null)
       {
         throw new ArgumentException(
           "expected property or field expression.");
       }
       var compiled = expression.Compile();
       var value = compiled();
       if( value == null)
       {
         throw new ArgumentNullException(body.Member.Name);
       }
     }
     public static void ThrowIfNullOrEmpty(Expression<Func<String>> expression)  
     {
        var body = expression.Body as MemberExpression;
        if (body == null)
        {
          throw new ArgumentException(
            "expected property or field expression.");
        }
        var compiled = expression.Compile();
        var value = compiled();
        if (String.IsNullOrEmpty(value))
        {
          throw new ArgumentException(
            "String is null or empty", body.Member.Name);
        }
      }
   }

}

Used like this:

public void Proc(object parameter1, object parameter2, string string1)
{
    Validator.ThrowIfNull(() => parameter1);
    Validator.ThrowIfNull(() => parameter2);
    Validator.ThrowIfNullOrEmpty(() => string1);
    // Main code.
}

Answer 3

Second one seems more elegant way of handling the same. In this case you can put restriction on every managed object.

internal static void ThrowIfNull(this object o, string paramName)
{
       if (o == null)
        throw new ArgumentNullException(paramName);
}

Answer 4

I'd use internal static void ThrowIfNull<T>(this T o, string paramName) where T : class. I won't use internal static void ThrowIfNull(this object o, string paramName) because it might do boxing.

Answer 5

I would do this way to avoid hardcoding parameter names. Tomorrow it can change, and you have more work then:

public static void ThrowIfNull<T>(this T item) where T : class
{
    if (item == null)
        return;

    var param = typeof(T).GetProperties()[0];
    if (param.GetValue(item, null) == null)
        throw new ArgumentNullException(param.Name);
}

And call it:

public void Proc(object parameter)
{
    new { parameter }.ThrowIfNull(); //you have to call it this way.

    // Main code.
}

The performance hit is trivial (on my mediocre computer it ran for 100000 times just under 25 ms), much faster than Expression based approach seen typically

ThrowIfNull(() => resource);

One such here. But surely don't use this if you cant afford that much hit..

You can also extend this for properties of objects.

new { myClass.MyProperty1 }.ThrowIfNull();

You can cache property values to improve performance further as property names don't change during runtime.

See this question additionally: Resolving a parameter name at runtime