When should I guard against null? [closed]

Question

When should I guard against null arguments? Ideally, I would guard against null everywhere, but that gets very bloated and tedious. I also note that people aren't putting guards in things like AsyncCallbacks.

To keep from annoying other people with lots of unidiomatic code, is there any accepted standard as to where I should guard against null?

Thanks.

Answer 1

When should I guard against null arguments?

I will assume you're talking about null arguments passed to public methods or constructors of code you've written. Note that you also might have to "guard" against null whenever you call any external dependency that might return null, unless your code can gracefully handle those cases.

You should guard against null in any public method (including constructors and property setters) that you expose to a user where the null value doesn't have a useful and explicit meaning. If a null value doesn't mean something special to your code (e.g. end of array, "unknown", etc) then you should not accept that value, and should throw an ArgumentNullException instead.

This rule isn't unique to null either. You should always check arguments passed to your public methods.

For example, say you are writing some sort of web service method that does takes a user Id, and does something to a user (e.g. delete them). You should verify that it is a valid user Id before your method does anything else with that user. Another example is if you are writing a public method that takes an index to a collection or array. You should check up front that the index is within the allowed range - that the index isn't bigger than the collection, or less than zero.

I also note that people aren't putting guards in things like AsyncCallbacks.

If you know that your method pre-conditions are vigilantly kept by the arguments passed to your methods (because you throw exceptions if they aren't), then you are free to skip these checks in your private and internal methods, or private and internal classes.

But as you pointed out, you must still be careful not to trust any return value from an API you didn't write, or any value passed in to your callback methods. Treat them as "dirty", and assume they can be null or invalid values.

that gets very bloated and tedious

Specifying and keeping track of pre-conditions for your public methods isn't bloat - it is compiled documentation. It is how you make sure your code is right. It is how users of your code are told up front that they did something wrong. Letting this fail in the middle of your method (or maybe in another method in some vaguely related class) makes it that much harder to debug your issue.

This may not seem like a big deal now. But once you start getting complaints from customers with a NullReferenceException 5 levels down in your stack, 20 method calls later, then I think you'll start to see the benefits :)

To keep from annoying other people with lots of unidiomatic code, is there any accepted standard as to where I should guard against null?

Typically people just write if ... throw code at the top of their method. This is the most idiomatic syntax, and very easy to understand even for beginners. Sort of like parens in Lisp, once you're to used that pattern you are able to skim it very quickly, without thinking about it.

You could make it faster to write these checks by using Visual Studio Code Snippets.

You could shorten this code a little by using or building some shared code that supports assertion syntax. Instead of if ... throw syntax, you would write a line like Assert.NotNull(arg1, "arg1");. If you want some inspiration you could look at the NUnit framework's assertions and constraints.

You might also want to look into the Code Contracts API. It is designed for checking pre-conditions, post-conditions, and invariants (which are the formal names for these "guard conditions"). It can also move some of this verification from run time to compile time, so you can find out you've made mistakes before even running your program. I haven't really looked at it, but it might also give you more concise syntax. Edit: Also see Pencho Ilchev's answer for a short example of using part of this API.

Answer 2

One approach which I have used a lot is the null object pattern. For example, if a have a factory class which returns different implementations of an interface based on an argument, and the provided argument is not mapped to any of the implementations, I would return a NullObject, e.g.

   public interface IFoo{
         void Bar();
   }
   public class NullFoo{
       public void Bar(){
          //null behaviour 
       }
   }
   public class FooFactory{
        public IFoo CreateFoo(int i){
              switch(i){
                  case 1:
                  return new OneFoo();
                  break;
                  case 2:
                  return new TwoFoo();
                  break;
                  default:
                  return new NullFoo();
                  break;
              }
        } 
   }

When I want get an IFoo from CreateFoo, I don't have to check whether the returned object is null.

Obviously, this is just one of the many approaches. There is no "one size fits all" since null can mean different things.

Another way to guard against null arguments is to use CodeContract preconditions. e.g.

  public void Foo(Bar x){
      Contract.Requires<ArgumentNullException>( x != null, "x" );
      //access x
  }

Using Code Contracts allows you to run static code analysis against your code and catch bugs such as Foo(null). (more here)

One more why to do it would be to use a very simple generic extension method:

public static class Ex
{
    public static void EnsureNotNull<T>(this T t, string argName) where T:class
    {
        if(t == null)
        {
            throw new ArgumentNullException(argName);
        }
    }
}

Then you can check your arguments like this:

 public void Foo(Bar x, Bar y){
     x.EnsureNotNull("x");
     y.EnsureNotNull("y");
 }