Let’s have a chat about null

I’ll start with a question.

What does null mean?

Go ahead. Take a moment.

I’m guessing you’re thinking things like “the absence of value” or “a reference to nothing”.

If so, you’re not wrong, but let me rephrase the question and try again.

What does null mean to a business?

Now we’re getting somewhere.

That one is tricky to answer, right?

Maybe you don’t think the question makes any sense now.

Maybe you want to limit the scope of that question to a specific business before you can even attempt to answer it.

Precisely.

Null doesn’t inherently mean anything

In order to assign any meaning to null you have to look at it within a very narrow scope, but within that scope null can mean anything you want it to.

And that’s the problem. Null means nothing, but can be used for almost anything.

Null is also really scary

Many software developers are so scared of null that they’ll add hundreds or even thousands of lines of code to their solutions in the form of paranoid guard clauses checking specifically for null.

Does this look familiar to anyone?

public interface IFoo
{
    void DoSomething();
}

public interface IBar
{
    void DoSomething();
}

public class ContrivedExample
{
    public void DoSomething(IFoo foo, IBar bar)
    {
        if (foo == null)
        {
            throw new ArgumentNullException("Foo was null!");
        }

        if (bar == null)
        {
            throw new ArgumentNullException("Bar was null");
        }

        // Phew, we're safe. Now we can proceed to do something.

        foo.DoSomething();

        bar.DoSomething();
    }
}

We all want to write clean code!

We can all agree on that right?

Clean code is easy to read.

Clean code is self-documenting.

Clean code is expressive.

Is that clean code?

It seems to be reasonably easy to read.

Let’s think bigger picture.

What is that code expressing?

The only thing it’s expressing to me is that the author is extremely scared of null reference exceptions.

Before you even begin reading code within that method which expresses the desired behavior you have to read over two guard clauses which actually contain more lines of code than the actual desired behavior.

Let’s lock in that behavior with a unit test

So what is the behavior of that method?

Well, it’s supposed to invoke DoSomething() on foo and bar, right?

I’ll use Moq to pass a mock implementation of Foo and Bar to my method that is configured so I can verify that DoSomething() gets invoked the appropriate number of times on each dependency.

[TestClass]
public class ContrivedExampleTests
{
    [TestMethod]
    public void DoSomething_DoesSomething()
    {
        // Arrange
        var mockFoo = new Mock<IFoo>();

        mockFoo.Setup(mock => mock.DoSomething())
            .Verifiable();

        var mockBar = new Mock<IBar>();

        mockBar.Setup(mock => mock.DoSomething())
            .Verifiable();

        var contrivedExampleClass = new ContrivedExample();

        // Act
        contrivedExampleClass.DoSomething(mockFoo.Object, mockBar.Object);

        // Assert
        mockFoo.Verify(mock => mock.DoSomething(), Times.Once);

        mockBar.Verify(mock => mock.DoSomething(), Times.Once);
    }
}

We’re done right? We’ve tested the behavior of that method!

Wait.

Those guard clauses are code too. There’s nothing magical about those lines of code that make them not count as behavior. Their presence within our method body means those guard clauses are in fact part of the method’s behavior.

That method doesn’t only do what the author likely intended for it to do. It also throws ArgumentNullExceptions.

We can’t truly say we’ve locked in the behavior of the method without testing whether or not the method will throw an ArgumentNullException in the event either parameter is null.

We have to write a couple more tests if we want full coverage

[TestMethod]
[ExpectedException(typeof(ArgumentNullException))] // Assert
public void DoSomething_WhenFooIsNull_ThrowsArgumentNullException()
{
    // Arrange
    var mockBar = new Mock<IBar>();

    var contrivedExampleClass = new ContrivedExample();

    // Act
    contrivedExampleClass.DoSomething(null, mockBar.Object);
}

[TestMethod]
[ExpectedException(typeof(ArgumentNullException))] // Assert
public void DoSomething_WhenBarIsNull_ThrowsArgumentNullException()
{
    // Arrange
    var mockFoo = new Mock<IFoo>();

    var contrivedExampleClass = new ContrivedExample();

    // Act
    contrivedExampleClass.DoSomething(mockFoo.Object, null);
}

This is starting to feel silly.

What started as two innocent guard clauses inside a single method has added 30+ lines of code and two test methods to our solution.

And this is a fairly simple contrived example.

Did all of this ceremony and added code gain us anything?

Well, we certainly don’t have to worry about null any more!

Wait.

Do we?

Why were we scared of null in the first place?

Probably because it would cause an exception to be thrown.

Aren’t we still throwing an exception in the event of a null parameter?

Is an ArgumentNullException better than a NullReferenceException?

If we’re not expecting either exception, how are we going to handle either of them being thrown by that method?

If we did expect an exception, wouldn’t it have been just as easy to catch the NullReferenceException and save ourselves from all this added code and ceremony?

One more example

What do you think of this code?

public class AnotherContrivedExample
{
    private IFoo Foo { get; }

    private IBar Bar { get; }

    public AnotherContrivedExample(IFoo foo, IBar bar)
    {
        Foo = foo ?? throw new ArgumentNullException("Foo was null!");

        Bar = bar ?? throw new ArgumentNullException("Bar was null");
    }

    public void DoSomethingWithFoo()
    {
        Foo.DoSomething();
    }

    public void DoSomethingWithBar()
    {
        Bar.DoSomething();
    }
}

Better, right?

We’ve placed our guard clauses in the constructor rather than each method to save ourselves from copy-pasting them within each method.

As a bonus, we’ve also used throw expression syntax to save ourselves even more lines of code!

Nope. That code wasn’t any better.

In fact, we’ve even introduced an unintended side-effect. Can you see what it is? If not, keep reading.

Unit test time!

[TestClass]
public class AnotherContrivedExampleTests
{
    [TestMethod]
    [ExpectedException(typeof(ArgumentNullException))] // Assert
    public void Constructor_WillNullFoo_ThrowsArgumentNullException()
    {
        // Arrange
        var mockBar = new Mock<IBar>();

        // Act
        var anotherContrivedExample = new AnotherContrivedExample(null, mockBar.Object);
    }

    [TestMethod]
    [ExpectedException(typeof(ArgumentNullException))] // Assert
    public void Constructor_WillNullBar_ThrowsArgumentNullException()
    {
        // Arrange
        var mockFoo = new Mock<IFoo>();

        // Act
        var anotherContrivedExample = new AnotherContrivedExample(mockFoo.Object, null);
    }

    [TestMethod]
    public void DoSomethingWithFoo_DoesSomethingWithFoo()
    {
        // Arrange
        var mockFoo = new Mock<IFoo>();

        mockFoo.Setup(foo => foo.DoSomething())
            .Verifiable();

        var mockBar = new Mock<IBar>();

        mockBar.Setup(bar => bar.DoSomething())
            .Verifiable();

        var anotherContrivedExample = new AnotherContrivedExample(mockFoo.Object, mockBar.Object);

        // Act
        anotherContrivedExample.DoSomethingWithFoo();

        // Assert
        mockFoo.Verify(foo => foo.DoSomething(), Times.Once);
    }

    [TestMethod]
    public void DoSomethingWithBar_DoesSomethingWithBar()
    {
        // Arrange
        var mockFoo = new Mock<IFoo>();

        mockFoo.Setup(foo => foo.DoSomething())
            .Verifiable();

        var mockBar = new Mock<IBar>();

        mockBar.Setup(bar => bar.DoSomething())
            .Verifiable();

        var anotherContrivedExample = new AnotherContrivedExample(mockFoo.Object, mockBar.Object);

        // Act
        anotherContrivedExample.DoSomethingWithBar();

        // Assert
        mockBar.Verify(bar => bar.DoSomething(), Times.Once);
    }
}

There we go. 100% test coverage.

So what was that unintended side-effect?

Is it the fact that we still had to write two unit tests to lock in the behavior of the constructor throwing ArgumentNullExceptions?

Nope. That’s not it. Although that’s still a problem just like it was before.

We didn’t have to write those tests for each method, but we still had to write them once for the constructor.

Look back to the two methods in the class.

Notice how the class has two dependencies, but each method only uses one of those dependencies?

Now look at the two tests for those methods. Notice anything?

Each test method has to provide an implementation for both dependencies.

Even though each actual method only uses one of the dependencies.

If you don’t provide both dependencies, the constructor will throw an ArgumentNullException and your test will fail.

Those tests would be more expressive if they were written this way:

[TestMethod]
public void DoSomethingWithFoo_DoesSomethingWithFoo()
{
    // Arrange
    var mockFoo = new Mock<IFoo>();

    mockFoo.Setup(foo => foo.DoSomething())
        .Verifiable();

    var anotherContrivedExample = new AnotherContrivedExample(foo: mockFoo.Object, bar: null);

    // Act
    anotherContrivedExample.DoSomethingWithFoo();

    // Assert
    mockFoo.Verify(foo => foo.DoSomething(), Times.Once);
}

[TestMethod]
public void DoSomethingWithBar_DoesSomethingWithBar()
{
    // Arrange
    var mockBar = new Mock<IBar>();

    mockBar.Setup(bar => bar.DoSomething())
        .Verifiable();

    var anotherContrivedExample = new AnotherContrivedExample(foo: null, bar: mockBar.Object);

    // Act
    anotherContrivedExample.DoSomethingWithBar();

    // Assert
    mockBar.Verify(bar => bar.DoSomething(), Times.Once);
}

Now we can see that when we’re testing “DoSomethingWithFoo” that our “bar” dependency is explicitly passed as null.

Likewise, when we’re testing “DoSomethingWithBar” our “foo” dependency is explicitly passed as null.

You can tell from reading these two tests that not only should something happen with one dependency, but that nothing should happen with the other dependency!

But we can’t write our tests this way because of our paranoid constructor. Instead, we had to write our test methods in such a way that doesn’t express the fact that one of the two dependencies in either test is completely meaningless.

Reading tests is a great way to understand code, but these tests don’t give an accurate representation of the intended behavior.

Alright, so when exactly should we use null and how should we use it?

In a word, rarely.

As we established previously, null typically only has meaning in a very narrow scope

  • Keep the scope of your null usage as limited as possible.
    • Block scope is a good starting point. Try to avoid letting null escape the scope of the current block of code.
  • Null can definitely have a meaning within a procedure, but rarely has any meaning outside of that context.
    • This means, whenever possible, avoid returning null from any method.

Yes, limiting the scope of null in this manner is possible, but it might require some restructuring of code.

Here’s a simple example: If you’re writing a search method that returns a list of users, make it return an empty list instead of null if no users are found!

But what if you’re writing a method that searches for a single user by ID and that user is not found?

Surely you have to return null in that case, right?

If you didn’t return null, how would the calling code know that the user couldn’t be located?

The answer is to think higher level.

You’re not really “searching” for a user if you already know their ID. What you’re doing is actually issuing a command to the system to return that user to you.

If that user doesn’t exist, you should be surprised. And what does being surprised mean in code?

An exception of course!

Throw an exception if you can’t fulfill a command to return a resource by ID!

Your calling code will be far more expressive as a result, because not getting the resource you specifically asked for is definitely an exception.

Another general guideline

  • The closer you are to the user, the more likely it is that null can have a specific meaning
    • and the sooner you can prevent this null reference from propagating further down into the system the better

For example, in a Web-API project it’s quite likely that a user will fail to provide some required input which could result in a null parameter reaching one of your Controllers.

You can stop this null reference very close to the user by using route parameter constraints.

The user will receive a meaningful HTTP response to indicate the error, and your code will remain completely free of null guard clauses.

Parting thoughts

I’m not advocating that you never use null. Really, I’m not. I’m just advocating for careful consideration whenever doing so.

Null has a very specific meaning programmatically, but almost never has meaning at a business level. So always consider what context you’re operating in while writing code that involves null.

Most developers spend the majority of their time writing code that manages business objects, so passing null around rarely has meaning.

Null can create clarity within a procedure, even when this procedure lives within a business object. Conversely, null tends to be reduce clarity when it escapes that scope and gets passed between business objects.

If handling null is inevitable, do it as close to the user as possible to keep the rest of your code clean.

Remember, a NullReferenceException is a runtime exception. Turning NullReferenceExceptions into other types of exceptions through guard clauses might make you feel like you’re writing safer code, but it has no actual positive impact on the stability of your application.

Each time you investigate a NullReferenceException you should modify your code to prevent it from occurring in the future.

And how do you do that?

Well there’s no single answer to that question, but I hope I’ve at least convinced you that the solution isn’t adding guard clauses to the method which threw the NullReferenceException.

The appropriate way to fix a NullReferencException is to add code which prevents the null reference from propagating as close to the user as possible.

2 thoughts on “Let’s have a chat about null

  • August 22, 2017 at 6:21 pm
    Permalink

    If one validates ArgumentNullException behavior via test, one should also validate NullReferenceException behavior. # of unit tests does not argue one way or the other.

    In practice, I view most guards as a more maintainable form of “Trace.Assert(x != null);”. As such, there is relatively low ROI for explicitly testing them. That is a business reality. Validating ArgumentNullException behavior via testing in most applications is a low-ROI solution looking for a problem.

    Relying on NullReferenceExceptions has risks. Programmers writing the functionality you call may use ?. or ?? … in that case, nulls result in different functionality. Every parameter to a public method should be checked. More specifically, every parameter at a boundary should be checked. Don’t rely on some other piece of code throwing a null reference exception as a side effect. If it is part of the contract, then check it.

    Adding “?? throw new ArgumentNullException(nameof(X))” provides some significant value.

    It allows code analysis to be more efficient. Resharper is more effective, for example, when provided the null reference checks.

    The ArgumentNullException places a more meaningful exception earlier in the code. I don’t have to play detective trying to figure out what actually caused null reference exception nested several levels deep in private (unguarded) method calls that have no line numbers in release mode.

    Null is a useful value in and of itself. Any SQL or mongo or redis programmer can tell you this. Null is so useful, C# now has nullable types and operators like ?. or ?? to powerfully make use of it. Null is omnipresent. We don’t go around initializing strings to string.Empty (as a surrogate for null). Null is default(T) for classes. Accept that. Not using null is basically throwing a big chunk of the useful of C# and SQL and REDIS and Mongo (and so forth) out of the window.

    Returning empty lists or throwing exceptions as a surrogate for a null is unmaintainable obfuscation. Null is simple and clean,

    Should I throw an exception if my record was not found? No! I write scale-able apps… maybe another instance deleted the record just now or maybe it just expired from the cache. _I_ am not “surprised” by the null nor do I find it exceptional. Null is naturally what SQL or mongo or REDIS returns. We should not use exceptions as surrogate for returning a valid value. Null is what I want in my action so I can do “result == null ? Ok(result) : NotFound()”. Exceptions are for exceptional cases (for example, the database failed).

    Reply
    • August 27, 2017 at 7:18 am
      Permalink

      Hi John, thanks for reading my post and taking the time to reply.

      Let me clarify. I’m not advocating for never using null. My summary was basically just be careful when using it and only use it when it has a real meaning to do so in context. Null rarely has a business meaning, so if it’s being returned from business logic code, I simply am stating there’s probably a way to handle the same behavior without null that might be more expressive. Null is perfectly fine to be used within a procedure, or even returned from code that is separated from business logic such as data access like you mentioned.

      I think what you’re getting hung up on is the separation of business logic from everything else. What I consider business logic isn’t data access, or view rendering, it’s the code in between that manipulates the business objects. Nobody has ever placed an order for null items, traded null stocks, bought a null movie ticket, etc. That’s what I mean by business logic. When you’re manipulating business objects, it makes more sense to have empty lists than null lists and to thrown Exceptions instead of returning null references.

      Your data access code is almost certainly going to return null. Like you mentioned, this is a default behavior of most data access libraries. EntityFramework has methods like “FirstOrDefault” specifically to return a null reference if a DbSet doesn’t contain the resource being requested. This is perfectly fine, however, when you turn these database entities into business objects, don’t pass on the null reference returned from the database. Convert it into something that has a business meaning.

      If you’re directly returning the database entities up through the system then the Null references are unavoidable, so I suppose I should have put a big disclaimer on my post that this thought is really only applicable in situations wherein you’re mapping the database entities to business objects before returning them up the call chain.

      Wouldn’t you agree that a method signature like this:

      public Order GetOrderById(int id) …

      … should return an Order object?

      Of course it should. Which is why anything else, from a business perspective, is an Exception.

      If you return null instead, that method signature no longer tells the full story. Sure, any reference type can be null, but does that mean you have to test for null whenever you get any value returned from ANY method returning a reference type? If so, your code is going to be absolutely littered with null checks. If not, how do you know when to check for null and when not to? There is no way to know other than by reading the code that defines the method being invoked. That’s hardly easy to use and maintain. Working on a code base like that, I’m not going to be able to trust the contract defined by my method signatures, and I’m going to have to read every line of code in every method I invoke just in case it’s designed to return null. Avoiding null in these business layers alleviates this code bloat and required reading. It makes your contracts reliable and easy to understand without having to dig into the implementation.

      Notice how Entity Framework for example has methods called “FirstOrDefault” which indicate by the very name of the method that null is a possible return value? That’s OK, it’s implied by the method name, but something like “GetOrder” returning null is instead very misleading.

      Hope that clears up my views a bit.

      And yes, you’re right, each NullReferenceException that doesn’t get caught becomes a research project. But that’s how it should be. Catching the null reference and converting it to another Exception type doesn’t save you from having to track it down. You can never fix a null reference bug from within the code that has already received the null reference. You always have to chase it up the call chain and find out when it becomes null.

      Reply

Leave a Reply

Your email address will not be published. Required fields are marked *