Search code examples
c#nullreferenceexceptionpex

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


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....


Solution

  • 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.