When to check for nulls

Question

This is kinda an open ended question but I'm trying to sharpen my skills with good practices in exception handling specifically when to check for nulls in general.

I know mostly when to check for nulls but I have to say half the time I don't and that's bothering to me. I know ints cannot be set to null unless you use a nullable int. I know strings can be set to null OR empty hence you can check for IsNullOrEmpty.

Obviously in constructors you want to add your explicit checks there also. That's a given. And that I think you should check for null anytime you're passing in an array, generic object, or other objects into a method that can be set to null basically right?

But there is more here to exception handling. For instance I don't know when to always check and throw a null ref exception explicitely in code. If I've got incoming params, usually it's pretty straight forward but there are always cases where I ask myself, is an explicit null throw needed?

I don't really have specific examples but wondering if there's a good reference out there that really talks about exception handling in reference to when to throw them (inside methods, classes, you name it).

Answer 1

You shouldn't throw NullReferenceException. If it is an argument that is null throw ArgumentNullException instead.

I prefer to check for null for all reference types parameters of public/protected methods. For private methods, you may omit the check if you're sure all calls will always have valid data.

Answer 2

Unless you're using Code Contracts, I'd say it's good practice to check arguments for any public/protected member - as well as explicitly documenting whether they can or can't be null. For private/internal methods it becomes a matter of verifying your own code rather than someone else's... it's a judgement call at that point.

I sometimes use a helper extension method for this, so I can write:

public void Foo(string x, string y, string z)
{
    x.ThrowIfNull("x");
    y.ThrowIfNull("y");
    // Don't check z - it's allowed to be null
    // Method body here
}

ThrowIfNull will throw an ArgumentNullException with the appropriate name.

By checking explicitly before you make any other calls, you know that if an exception is thrown, nothing else will have happened so you won't have corrupted state.

I admit I'll omit such checks where I know for certain that the first call will do the same checks - e.g. if it's one overload piggybacking onto another.

With Code Contracts, I'd write:

public void Foo(string x, string y, string z)
{
    Contract.Requires(x != null);
    Contract.Requires(y != null);

    // Rest of method here
}

This will throw a ContractException instead of ArgumentNullException, but all the information is still present and no-one should be catching ArgumentNullException explicitly anyway (other than possibly to cope with third party badness).

Of course the decision of whether you should allow null values is an entirely different matter. That depends entirely on the situation. Preventing nulls from ever entering your world does make some things easier, but at the same time nulls can be useful in their own right.

Answer 3

My rules of thumb for when to check for nulls are:

• Always check the arguments passed to a public/protected method of any class.
• Always check the arguments to constructors and initialize methods.
• Always check the arguments in an indexer or property setter.
• Always check the arguments to methods that are interface implementations.
• With multiple overloads, try to put all argument preconditions in a single method that other overloads delegate to.

Early notification of null arguments (closer to the point of error) are much better than random NullReferenceExceptions that occur far from the point of the error.

You can use utility methods to clean up the typical if( arg == null ) throw new ArgumentNullException(...); constructs. You may also want to look into code contracts in C# 4.0 as a way to improve your code. Code contracts perform both static and runtime checks which can help identify problems even at compile time.

In general, writing preconditions for methods is a time consuming (and therefore often omitted) practice. However, the benefit to this type of defensive coding is in the hours of potentially saved time debugging red herrings that occur because you didn't validate your inputs.

As a side note, ReSharper is a good tool for identifying potential access to arguments or local members that may be null at runtime.

Answer 4

If you can't deal with an exception at a given level of code, don't catch it. If this means that your whole application comes down, then it's probably best that you got the warning rather than having a catch-all which puts your software into an unknown / unpredictable state.

One note: when setting fields with constructors, set your fields to readonly. Then you can be sure that your assertion about the reference still stands for the lifetime of the object.

Answer 5

The earlier the better. The sooner you catch the (invalid) null, the less likely you'll be throwing out in the middle of some critical process. One other thing to consider is using your properties (setters) to centralize your validation. Often I reference my properties in the constructor, so I get good reuse on that validation.

class A
{
  private string _Name
  public string Name
  {
    get { return _Name; }
    set 
    {
      if (value == null)
         throw new ArgumentNullException("Name");
      _Name = value;
    }
  }

  public A(string name)
  {
     //Note the use of property with built in validation
     Name = name;
  }
}

Answer 6

Depends on your type of method/api/library/framework. I think its ok for private methods that they dont check for null values unless they are kind of assertion methods.

If you programm defensive and litter your code with "not null"-tests or "if"-statements your code will be unreadable.

Define the areas where a client can mess up your code by submitting wrong/empty/null arguments. If you know where, you can draw the line where to do your null checks. Do it at your api entry points just like the security works in real life.

Imagine you would be bothered every minute to show your ticket in a cinema every movie would suck.

Answer 7

I prefer using static template methods for checking input constraints. the general format for this would be something like:

static class Check {
    public static T NotNull(T arg) {
        if( arg == null ) throw new ArgumentNullException();
    }
}

The benefit to using this is that your method or constructor can now wrap the first use of the argument with Check.NotNull() as exampled here:

this.instanceMember = Check.NotNull(myArgument);

When I get to .Net 4, I'll likely convert to code contracts, but until then this works. BTW, You can find a more complete definition of the Check class I use at the following url:

http://code.google.com/p/csharptest-net/source/browse/trunk/src/Shared/Check.cs