What is the best practice in case one argument is

Question

when validating methods' input, I used to check if the argument is null, and if so I throw an ArgumentNullException. I do this for each and every argument in the list so I end up with code like this:

 public User CreateUser(string userName, string password, 
                            string Email, string emailAlerts, 
                            string channelDescription)
    {

        if (string.IsNullOrEmpty(userName))
            throw new ArgumentNullException("Username can't be null");

        if (string.IsNullOrEmpty(Email))
            throw new ArgumentNullException("Email can't be null");
       //etc, etc, etc
    }

Is this OK? Why should I do this? Would it be ok if I simply group all the checks and return a null value instead of throwing the exception? What is the best practice to address this situation?

PS: I want to change this, because with long methods, it gets very tedious to do so.
Ideas?

Answer 1

And for the C# 3.0 developers amongst us a great way to encapsulate this null checking is inside an extension method.

public void Foo(string arg1, int? arg2)
{
  arg1.ThrowOnNull();
  arg2.ThrowOnNull();
}

public static class extensions
{
    public static void ThrowOnNull<T>(this T argument) where T : class
    {
        if(argument == null) throw new ArgumentNullException();
    } 
}

And if you wanted you could always overload that to take an argument name.

Answer 2

An approach which I use and I may have picked up from the NHibernate source is to create a static class Guard, used as follows:

public void Foo(object arg1, string arg2, int arg3)
{
    Guard.ArgumentNotNull(arg1, "arg1");
    Guard.ArgumentNotNullOrEmpty(arg2, "arg2");
    Guard.ArgumentGreaterThan(arg3, "arg3", 0);
    //etc.
}

public static class Guard
{
    public static void ArgumentNotNull(object argument, string parameterName)
    {
        if (parameterName == null)
            throw new ArgumentNullException("parameterName");

        if (argument == null)
            throw new ArgumentNullException(parameterName);
    }
    //etc.
}

This cuts down a lot of the chaff at the beginning of methods and it performs well.

Answer 3

I would stick with your original approach, except for just passing in the parameter name. The reason is that once you start writing those helper procedures it becomes an issue when everyone starts using different conventions for how they write the helper procedures. When someone looks over your code, they now have to check to make sure you've written the helper procedure correctly when debugging your code.

Keep checking each argument separately, though youor fingers grow weary from typing Grasshopper :) Your followers will bless you when they get the unexpected ArgumentException and are saved from a debugging run just to determine which argument failed.

Answer 4

Make an ArgChecker class with something like this

  ArgChecker.ThrowOnStringNullOrEmpty(userName, "Username");

where ThrowOnStringNullOrEmpty is

  public static void ThrowOnStringNullOrEmpty(string arg, string name)
  {
      if (string.IsNullOrEmpty(arg))
        throw new ArgumentNullException(name + " can't be null");
  }

You could also try to process a list of arguments using a params arg, like:

  public static void ThrowOnAnyStringNullOrEmpty(params string[] argAndNames)
  {
       for (int i = 0; i < argAndName.Length; i+=2) {
          ThrowOnStringNullOrEmpty(argAndNames[i], argAndNames[i+1]);
       }
  }

and call like this

  ArgChecker.ThrowOnAnyStringNullOrEmpty(userName, "Username", Email, "email");

Answer 5

A small improvement to Lou's answer would be to use a hashtable instead, it means it checks objects aswell as just strings. Also just nicer to populate and handle in the method:

public static class ParameterChecker
{
    public static void CheckForNull(Hashtable parameters)
    {
        foreach (DictionaryEntry param in parameters)
        {
            if (param.Value == null || string.IsNullOrEmpty(param.Value as string))
            {
                throw new ArgumentNullException(param.Key.ToString());
            }
        }
    }
}

As you would use like:

public User CreateUser(string userName, string password, string Email, string emailAlerts, string channelDescription)    
{
    var parameters = new Hashtable();
    parameters.Add("Username", userName);
    parameters.Add("Password", password);
    parameters.Add("EmailAlerts", emailAlerts);
    parameters.Add("ChannelDescription", channelDescription);
    ParameterChecker.CheckForNull(parameters);

    // etc etc
}

Answer 6

My first advice to you is get ReSharper. It will tell you when there is a problem of possible null values, and when there is no need to check for them, and with the click of a mouse will add checking. Having said that...

You don't have to check for int or bool, which cannot be null.

Strings can be checked with string.IsNullOrEmpty()...

If you still decide that you want to check each and every parameter, you can either use the Command design pattern, and reflection, but your code will be unnecessarily clunky, or use the following and call it for each method: private myType myMethod(string param1, int param2, byte[] param3) { CheckParameters("myMethod", {param1, param2, param3}); // rest of code...

And in your utility class put this:

///<summary>Validates method parameters</summary>
///... rest of documentation
public void CheckParameters(string methodName, List<Object> parameterValues) 
{
    if ( string.IsNullOrEmpty(methodName) )
       throw new ArgumentException("Fire the programmer! Missing method name", "methodName"));

    Type t = typeof(MyClass);
    MethodInfo method = t.GetMethod(methodName);
    if ( method == null )
       throw new ArgumentException("Fire the programmer! Wrong method name", "methodName"));
    List<ParameterInfo> params = method.GetParameters();
    if ( params == null || params.Count != parameterValues.Count )
       throw new ArgumentException("Fire the programmer! Wrong list of parameters. Should have " + params.Count + " parameters", "parameterValues"));

    for (int i = 0; i < params.Count; i++ )
    {
            ParamInfo param = params[i];
            if ( param.Type != typeof(parameterValues[i]) )
                throw new ArgumentException("Fire the programmer! Wrong order of parameters. Error in param " + param.Name, "parameterValues"));
            if ( parameterValues[i] == null )
                throw new ArgumentException(param.Name + " cannot be null");
    }
} // enjoy