Deeply Nested Null Checking in C# verses Assert with no nesting

One of the code smells that particularly bothers me (though I often find myself doing it anyhow) is when I defensively program against nulls in C# (though could be any other language).  That is, I do something like the following

var rec = getRecord(..);
if (rec != null)
{
   var rec1 = getAnotherRecord(..);
   if (rec1 != null) 
   {
       rec2 = getAThridRecord(..);
       if (rec2 != null)...

The code gets ugly quick and the nesting does not help the readability, and if anything, hurts it.

Today, while using a daily build of Resharper7, I noticed that when I asked resharper to do the null check for me, instead of doing the above, it did the following:

var rec = getRecord(..);
Debug.Assert(rec != null,"rec != null");

var rec1 = getAnotherRecord(..);
Debug.Assert(rec1 != null,"rec1 != null");

var rec2 = getAThirdrRecord(..);
Debug.Assert(rec2 != null,"rec2 != null");

I think I like this better.  Still to early to decide such a big shift in my coding style.

What do you think?

About Peter Kellner

Peter is a software professional specializing in mobile and web technologies. He has also been a Microsoft MVP for the past 7 years. To read more about Peter Kellner and his experience click here. For information about how Peter Kellner might be able to help you with your project click here.

Follow me:


Comments

  1. If the normal behavior is to not be null, unless you can “recover” from a null and return something sensible, I would go for a guard for each of these:

    var stuff = GetStuff();
    if (stuff == null) { throw new ArgumentException(“Stuff should not be null”); }

    Regarding Anders, he probably was referring to Tony Hoare, who introduced the null reference in programming, and called it his “Billion-dollar mistake” much later.

    Incidentally, F# doesn’t have a Null (well, it exists because it needs to talk to C#…) so that class of problems is inexistent there – by design ;)

  2. David Williams says:

    If you are using Resharper (as you state) then I have found a better solution is to mark the procedures with the NotNull attribute. This acomplishes two different things: The consuming code will not need the null checks and Resharper does a static analsys and will tell you if you allow a code path that allows the null to be returned. All at coding time – thereby removing (most – I still tend to check at boundary layers anyhow) of the checks altogether.

  3. The scenario I’m really referring to is when something technically could be null but never actually should be. For example, when I am being passed in a primary key and I’m doing a GetRecord(Id) kind of thing. If the return is null, that is a logic mistake some place upstream that should not have happened so a throw is warranted. I remember hearing anders once say that the biggest thing he would correct in c# if he could start again was to have non-nullable objects, then we would not be in this soup.

  4. Michael Stortz says:

    These two blocks don’t do the same thing. The structure of block one implies that a getRecord call might well return null (and proceeds only if not null), while block 2 treats a null as an error. If you don’t want to put the “null = error” logic in the getRecord call (null is OK in some scenarios, not OK in others), I would say “var rec = getRecord(…); if (rec == null) throw new Exception(“Record can not be null”);”

  5. Debug.Assert should only be used when you believe that the asserted condition is always true. The code is only evaluated when debugging; it is removed by the compiler in a release build.

    The nested if statements are necessary when you anticipate that the null condition may occur under some circumstances (such as invald user input, etc.) and the correct action when it does occur is to do nothing.

  6. Only use it for debugging. Compiler will ignore those lines unless you set the DEBUG compiler directive

    http://msdn.microsoft.com/en-us/library/system.diagnostics.conditionalattribute.aspx

    Consider creating a Guard class to check your method’s parameters.

    http://stackoverflow.com/questions/299439/net-guard-class-library

Follow

Get every new post delivered to your Inbox

Join other followers: