Null checking

Introduction – The Basics

A NullReferenceException is an incredibly common mistake and probably the first problem new developers encounter. If you have a reference to an object, but the object is null, you cannot call instance methods on it without it throwing an exception.

So in C#, you could have 

Dog myDog = null;
myDog.Bark();

Which will throw an exception because myDog isn’t initialised.

Dog myDog = new Dog();
myDog.Bark();

This is fine because a dog has been initialised and holds a reference to an object.

If you allow the possibility of nulls, then whenever you want to call a method, you end up checking for null.

if (myDog !=null)
   myDog.Bark();

More concise syntax involves using a question mark which will conditionally execute if it is not null such as:

myDog?.Bark()

So the question mark acts as the IF statement but is a more concise way of expressing it.

New “Nullable Reference Types & Improved Design

A cleaner, and safer design, if you can, is to never allow nulls, then you never have to check for them. In the latest versions of C#, you can make it so the compiler will assume things shouldn’t be null, unless you explicitly specify they can be; see Introducing Nullable Reference Types in C# – .NET Blog (microsoft.com).

Our Problem

I work on some software which is over 10 years old using an older version of .Net. So things can be null, and are often designed for null. We noticed that newer developers (seem to be common for our Indian developers for some reason), seem to put null checks everywhere, regardless if the reference can be null or not. This makes the code much harder to read, and possibly debug, because you are writing misleading code, and adding extra code branches that would never execute.

In a recent code review, the developer added an if statement to check for null

if (user == null)
   return;

So if it got past this code, user cannot be null, yet for every single statement afterwards, he added the question mark to check for null! eg

var preferences = user?.Preferences;

Although it’s also bad design to chain loads of properties together, it is sometimes necessary. Trying to be concise and adding the questionmark to check for nulls can be hard to understand what is actually executed. Combined with LINQs FirstOrDefault, you get even less clarity. FirstOrDefault will return the first item in a list, and if there are no matches; it will return null. So when you see all the conditionals and a method like FirstOrDefault, then when you glance at the statement, it looks very likely it will return null.

var result = value?.Details?.FirstOrDefault() is Observation

So “result” is null if: value is null; details is null; or details contains no items.

A longer example is the following:

if (recordData != null && (_templateDataSource?.EventLinks?.ContainsKey(recordData) ?? false))

Due to the null checks, they have added the “null coalescing operator” (??) and set to false when null. However, none of this data should have been null in their code, so it should be:

if (_templateDataSource.EventLinks.ContainsKey(recordData))

The Lead Developer made a good point that if this data was null, it would be a major bug, but if you put the null checks, then the logic gets skipped and a bug is hidden. It would be preferable to actually crash so you know something is very wrong.

Lead Developer

Should this ever be null? Or the EventLinks on it?
Remember, all these null safety checks could allow the software to continue on invalid data conditions and produce wrong answers instead of crash.
Please re-check ALL the ones you have added in these changes, and if something should never be null remove it.
Unless you are planning to test the behaviour of this when null is passed in and that it is correct.

When so many objects can actually be null, it is easy to miss a check. There was one code review where I questioned the original design which seemed to have excessive null checks (which was implemented by the same author of this new change). His new change was to add a null check that was missed. After making some dramatic changes based on my feedback, he ironically removed his new null check, and thus reintroduced the issue he was attempting to fix!

Developer: "have added a null check to prevent the issue."
Me: "then it will crash because you haven't added null checks"

Another good example of having excessive null checks, but somehow still missing them:

context.NotGiven = resource.NotGiven;
context.Date = Helper.GetDateTimeFromString(resource.Date);
context.IdentifierSystem = resource.Identifier?.FirstOrDefault().System;
context.IdentifierValue = resource.Identifier?.FirstOrDefault().Value;
context.Status = resource.Status;
Coding productCode = resource.productCode?.Coding?.FirstOrDefault(x => x.System == Helper.SystematicInfoUrl);
context.productCode = new FilerCommon.Coding(productCode?.System, productCode?.Code, productCode?.Display);
Coding productProcedureCode = (resource.GetExtension(Helper.ProductProcedureUrl)?.Value as CodeableConcept)?.Coding?.FirstOrDefault(x => x.System == Helper.SystematicInfoUrl);
context.productProcedureCode = new FilerCommon.Coding(productProcedureCode?.System, productProcedureCode?.Code, productProcedureCode?.Display);
Coding site = resource.Site?.Coding?.FirstOrDefault(x => x.System == Helper.SystematicInfoUrl);
context.Site = new FilerCommon.Coding(site?.System, site?.Code, site?.Display);
Coding route = resource.Route?.Coding?.FirstOrDefault(x => x.System == Helper.SystematicInfoUrl);
context.Route = new FilerCommon.Coding(route?.System, route?.Code, route?.Display);
Coding explanation = (context.NotGiven.HasValue && context.NotGiven.Value ? resource.Explanation?.ReasonNotGiven : resource.Explanation?.Reason)?.FirstOrDefault()?.Coding?.FirstOrDefault(x => x.System == Helper.SystematicInfoUrl);
context.Explanation = new FilerCommon.Coding(explanation?.System, explanation?.Code, explanation?.Display);
context.Quantity = new FilerCommon.Quantity(resource.Quantity?.Code, resource.Quantity?.Value, resource.Quantity?.Unit);            
context.LotNumber = resource.LotNumber;
context.Manufacturer = GetManufacturerName(context, resource);
context.OrganisationDetail = GetOrganizationDetails(context, resource);

Some of the FirstOrDefault() then call a property without a null check:

FirstOrDefault().System

Also, because of the null checks when they are setting “site”, it looks like it can be null. So when they set “context.Site”, they have null checks in the constructor for Coding. However that means they are passing null into the constructor which means it probably is an invalid object.

Null flavours

I thought the idea of “Null flavor” is very interesting. NullFlavor – FHIR v4.0.1 (hl7.org) Null means “nothing” but there’s different meanings within; or the reason why it is missing. Some examples are:

• missing, omitted, incomplete, improper
• Invalid
• not been provided by the sender due to security, privacy or other reasons
• Unknown
• not applicable (e.g., last menstrual period for a male)
• not available at this time but it is expected that it will be available later.
• not available at this time (with no expectation regarding whether it will or will not be available in the future).
• The content is greater than zero, but too small to be quantified.

Closing Words

Null checks are one of the simplest concepts in object oriented programming, so it’s bizarre how we are seeing many modern programmers struggling to understand when to use them. Even when they find a bug in their code, they still fail to learn their lesson and continue to write bad code with inappropriate null checks. A better design is to avoid the possibility of nulls by always dealing with an object reference (even if the object is one that simply represents “null”).