Error Handling – Should You Check for Null if Not Expected?

error handlingnull

Last week, we had a heated argument about handling nulls in our application's service layer. The question is in the .NET context, but it will be the same in Java and many other technologies.

The question was: should you always check for nulls and make your code work no matter what, or let an exception bubble up when a null is received unexpectedly?

On one side, checking for null where you are not expecting it (i.e. have no user interface to handle it) is, in my opinion, the same as writing a try block with empty catch. You are just hiding an error. The error might be that something has changed in the code and null is now an expected value, or there is some other error and the wrong ID is passed to the method.

On the other hand, checking for nulls may be a good habit in general. Moreover, if there is a check the application may go on working, with just a small part of the functionality not having any effect. Then the customer might report a small bug like "cannot delete comment" instead of much more severe bug like "cannot open page X".

What practice do you follow and what are your arguments for or against either approach?

Update:

I want to add some detail about our particular case. We were retrieving some objects from the database and did some processing on them (let's say, build a collection). The developer who wrote the code did not anticipate that the object could be null so he did not include any checks, and when the page was loaded there was an error and the whole page did not load.

Obviously, in this case there should have been a check. Then we got into an argument over whether every object that is processed should be checked, even if it is not expected to be missing, and whether the eventual processing should be aborted silently.

The hypothetical benefit would be that the page will continue working. Think of a search results on Stack Exchange in different groups (users, comments, questions). The method could check for null and abort the processing of users (which due to a bug is null) but return the "comments" and "questions" sections. The page would continue working except that the "users" section will be missing (which is a bug). Should we fail early and break the whole page or continue to work and wait for someone to notice that the "users" section is missing?

Best Answer

The question is not so much whether you should check for null or let the runtime throw an exception; it is how you should respond to such an unexpected situation.

Your options, then, are:

Throw a generic exception (NullReferenceException) and let it bubble up; if you don't do the null check yourself, this is what happens automatically.
Throw a custom exception that describes the problem on a higher level; this can be achieved either by throwing in the null check, or by catching a NullReferenceException and throwing the more specific exception.
Recover by substituting a suitable default value.

There is no general rule which one is the best solution. Personally, I would say:

The generic exception is best if the error condition would be a sign of a serious bug in your code, that is, the value that is null should never be allowed to get there in the first place. Such an exception may then bubble all the way up into whatever error logging you have set up, so that someone gets notified and fixes the bug.
The default value solution is good if providing semi-useful output is more important than correctness, such as a web browser that accepts technically incorrect HTML and makes a best effort to render it sensibly.
Otherwise, I'd go with the specific exception and handle it somewhere suitable.

Related Solutions

Error Handling – Null Pointers vs. Null Object Pattern

You wouldn't use a Null Object Pattern in places where null (or Null Object) is returned because there was a catastrophic failure. In those places I would continue to return null. In some cases, if there's no recovery, you might as well crash because at least the crash dump will indicate exactly where the problem occurred. In such cases when you add your own error handling, you are still going to kill the process (again, I said for cases where there's no recovery) but your error handling will mask very important information that a crash dump would've provided.

Null Object Pattern is more for places where there's a default behavior that could be taken in a case where object isn't found. For example consider the following:

User* pUser = GetUser( "Bob" );

if( pUser )
{
    pUser->SetAddress( "123 Fake St." );
}

If you use NOP, you would write:

GetUser( "Bob" )->SetAddress( "123 Fake St." );

Note that this code's behavior is "if Bob exists, I want to update his address". Obviously if your application requires Bob to be present, you don't want to silently succeed. But there are cases where this type of behavior would be appropriate. And in those cases, doesn't NOP produce a much cleaner and concise code?

In places where you really can't live without Bob, I would have GetUser() throw an application exception (i.e. not access violation or anything like that) that would be handled at a higher level and would report general operation failure. In this case, there's no need for NOP but there's also no need to explicitly check for NULL. IMO, those checks for NULL, only make the code bigger and take away from readability. Check for NULL is still the right design choice for some interfaces, but not nearly as many as some people tend to think.

Java Annotations – Check @Nonnull Parameters for Null

If you dont trust FindBug, it may be an option to use an assertion. This way you avoid the problems mentioned by User MSalters but still have a check. Of course, this approach may not be applicable if such a fail-fast approach is not feasible, i.e. you have to catch any exception.

And to further answer the question whether it is bad practice. Well, this is arguable but I would not think so. I consider this FindBug annotation as a light specification following the design by contract principle.

In design by contract you establish a contract between a method and its caller. The contract consists of preconditions that the caller agrees to fulfill when calling the method and a postcondition that in turn is fulfilled by the method after execution if its precondition is fulfilled.

One of the benefits of this development method is that you can avoid a so called defensive programming style (in which you check for all invalid parameter values explicitly etc.). Instead you can rely on the contract, and avoid redundant checks to increase performance and readibility.

Contracts can be used for runtime-assertion checking which follows the above mentioned fail-first principle in which you want your program to fail if a contract is broken, giving you a clue to identify the source of error. (A failed precondition means that the caller did something wrong, a failed postcondition indicates an implementation error of the given method)

Furthermore, contracts can be used for static analysis, which tries to draw conclusions about the source code without actually running it.

The @nonnull annotation can be seen as a precondition that is used for static analysis by the tool FindBugs. If you prefer an approach such as runtime-assertion checking, i.e. the fail first strategy, you should consider using assertion statements as built-in Java. Or maybe to adapt to a more sophisticated specification strategy using a behavioral interface specification language such as the Java Modeling Language (JML).

JML is an extension of Java in which specification is embedded in special Java Comments. An advantage of this approach is that you can use sophisticated specification, even using a development approach in which you only rely on specification rather than implementation details and use different tools that make use of the specification, such as the mentioned runtime-assertion checking tools, automated generation of unit-tests or documentation.

If you share my point of view of @nonnull being a (light) contract, it would be common practice to not add additional checks because the original idea behind this principle is to avoid defensive programming.

Best Answer

Related Solutions

Error Handling – Null Pointers vs. Null Object Pattern

Java Annotations – Check @Nonnull Parameters for Null

Related Topic