Avoiding Useless Checks

All of us are trying to write code that is safe. It’s not uncommon to come across something like that:

var accountRepository = GetAccountRepository();
if(accountRepository != null)
{
    var account = accountRepository.Get("username");
    if(account != null)
    {
        if(account.Roles != null && account.Roles.Contains("Admin"))
        {
            if(logger != null)
                logger.WriteLine("User is an administrator");
            Console.Write("User is in administrator role");
        }
    }
}

All is well, but wouldn’t it be cleaner if there weren’t so many checks for null? Something like this:

var accountRepository = GetAccountRepository();
if(accountRepository.Contains("username"))
{
    var account = accountRepository.Get("username");
    if(account.Roles.Contains("Admin"))
    {
        logger.WriteLine("User is an administrator");
        Console.Write("User is in administrator role");
    }
}

We’ve removed three checks for null and reduced the level of indentation by two. Not much, but still makes the code cleaner. But how do we ensure that we don’t get a NullReferenceException in the refactored piece of code?

Null Object

There’s a useful pattern for avoiding NullReferenceExceptions and reduce checks for nulls – Null object pattern. The idea is to never use nulls, but instead have an instance of object that is equivalent to null. Taking logging for example, let’s suppose we have a Logger object that writes messages to a file and it’s used like this:

public class Calculator
{
    private Logger logger = new Logger();

    public int Sum(int a, int b)
    {
        if(logger != null)
            logger.Write("Calculating the sum of {0} and {1}", a, b);
        return a + b;
    }
}

If we decide to temporarily disable logging, we can just assign null to the logger variable. But there’s a disadvantage – someone adding to the code and using the same logger variable might not notice or forget to add a null check. It’s also a slight violation of DRY principle to have all those null checks repeating over the code.

A much more object oriented and elegant way would be to derive from the Logger class and create a class called NullLogger, that would override the methods and just do nothing, like this:

public class NullLogger: Logger
{
    public override void Write(string format, params object[] parameters)
    {
        // do nothing
    }
}

Now, if we set the logger variable to a new instance of NullLogger, we achieve the same effect as setting it to null, but we can also remove the null checks, because the variable should not be null in any case.

Throwing Exceptions

It might also be more appropriate to throw an exception if a method cannot return a value. For example, if we have a method called GetConfiguration that reads configuration from a file and returns a Configuration object, it would be a better idea to throw a FormatException if config file cannot be parsed rather than returning a null.

Other Examples

When you’re manipulating strings, you can consider passing or returning a string.Empty instead of null. If you’re working with arrays or lists, you can use empty ones. I’ve seen checks like these:

IList aList = GetAList();
if(aList != null && aList.Count > 0)
{
    foreach(var element in aList)
    {
        // do something with the element
    }
}

The if statement is completely useless if GetAList returns an empty list, because then it is not equal to null and the foreach won’t execute if the element count is zero. So you can leave only the foreach loop in this case.

Conclusion

I realize that it’s important for the code to be as safe as possible, but it’s pretty easy to lose the idea of what the code really does in a pile of safety checks. Using Null objects or empty lists instead of nulls allows us to omit the checks for nulls and also makes code complexity lower and further refactoring easier.

CodeProject