Is allowing the possibility of NullReferenceExceptions to occur a Bad Thing?

Question

I'm playing with Pex and Moles and after running Pex found that nearly all the tests that Pex said failed were because NullReferenceExceptions were "allowed". Reading the Pex documentation, I came across the following:

If a higher-level component passes malformed data to a lower-level component, which the lower-level component rejects, then the higher-level component should be prevented from doing so in the first place.

So what the above is suggesting is that we should test for nulls before other methods/classes are called using something like:

if(foo == null)
   throw new ArgumentNullException("its null and this shouldn't happen")
else
   Bar(foo); //won't get a null reference exception here because we checked first...

IMHO checking for nulls all over doesn't appeal that much for performance and also code bloat reasons but I would like to hear what other people have to say....

Answer 1

Yes, you should validate your arguments before using them, IMO.

NullReferenceException should occur when an unanticipated null value is used. It should never be thrown explicitly, and indicates a problem at the level of the method which ends up throwing it, or something it's called.

ArgumentNullException indicates a bug in a method earlier in the call stack than the method throwing it. (Usually, but not always, the direct caller.)

The earlier you throw an exception indicating a problem, the easier it will be to pinpoint where the null value first crept in and the less likely it is that the "bad data" will have had a nasty effect elsewhere (e.g. overwriting a file ready to write data into it before realising that actually the data is null).

If you're confident in how you call your internal or private methods, it may be appropriate to not perform checking there, but for public methods I believe argument validation is pretty much always appropriate.

Answer 2

Yes, I would agree. A NullReferenceException is the result of an attempt to invoke a member on a variable that is a null reference. This means that there are no safe-guards in place to verify that it is a legal operation to invoke the member, and this is a bad thing in my eyes. You should always distrust input and verify that it is safe for you to use, before using it.

Answer 3

It is always a good practice to check for NULL when parameters are passed to a function which is exposing a service. Rest of NULL checking is down to common sense but it is useful to do and you can basically use a helper method to do it.

The most annoying NULL checking is with the strings. They can be quite nasty but I use an extension method to overcome that:

public static class StringExtensions
{
    public static string NullSafe(this string s)
    {
        return s ?? string.Empty;
    }
}

So you can use:

myString.NullSafe().ToUpper()

Answer 4

Even from a simple diagnostic perspective, what can you get more information about - A NullReferenceException or an ArguementNullException?

Take it a step further and take the stack trace out of the picture. Your two messages are probably:

NullReferenceException: "Object reference not set to an instance of an object."

• Something is null, somewhere and I probably did not expect it to be.
• I probably need to debug to find the error.

ArguementNullException: "System.ArgumentNullException: MyVariable cannot be null."

• I know that something null was passed to a method.
• Chances are, by the variable name, I can narrow it down to a smaller set of possible methods
• I still probably need to debug, but I should be able know some general places to put a breakpoint.

Also, check your syntax and read the info on what you are supposed to pass to the ArgumentNullException constructor.

throw new ArgumentNullException("its null and this shouldn't happen");

That is not right.

throw new ArgumentNullException("VariableName");

That is right.