Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
44  
Fox Mulder principle: Trust no one. –  Yannis Rizos May 6 '12 at 15:59
8  
@YannisRizos: that principle is a good one - it is so good that the creators of Java and C# let the language run time environment do this automatically. So normally there is no need to do explicit checks for null everywhere in your code in those languages. –  Doc Brown May 6 '12 at 16:26
3  
possible duplicate of Should a method validate its parameters? –  Loki Astari May 6 '12 at 19:06
    
I agree with tdammers answer. I believe checking for null is just wrong because you do not have a reasonable way to handle it. However in your scenario you can handle a section failing such as validating a user (or getting comments or whatever it may be) and having a "oh no an error has occurred" box. I personally log all the exceptions in my app and filter out certain exceptions like 404s and connection close unexpectedly –  acidzombie24 May 6 '12 at 21:40
1  
assert(foo != null, "foo is web control within the repeater, there's no reason to expect it to be null, etc, etc..."); –  zzzzBov May 6 '12 at 21:55

14 Answers 14

up vote 59 down vote accepted

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.
share|improve this answer
2  
+1 finally somebody nailed it. I was about to write an answer in those lines myself. –  Alex May 6 '12 at 19:46
    
+1 so hard. Great answer. –  acidzombie24 May 6 '12 at 21:34
    
What is the difference between specific exception and null in the grand scheme of things? The program still crashes if they are unhandled (i.e. there is no user interface for this case) –  Stilgar May 7 '12 at 6:26
    
@Stilgar for the enduser there is no difference. for the developper a specific exception like "database-server is not reachable" is more intuitive that "null pointer excepteion" –  k3b May 7 '12 at 7:16
9  
Failing hard and early means that there's less risk of things going more subtly wrong down the line, things like mis-authentication, corrupted data persisted to storage, information leakage, and other amusing things. –  Lars Viklund May 7 '12 at 12:14

IMHO trying to handle null values that you don't expect leads to overly complicated code. If you don't expect null, make it clear by throwing ArgumentNullException. I get really frustrated when people check if the value is null and then try to write some code that doesn't make any sense. The same applies to using SingleOrDefault (or even worse getting collection) when someone really expects Single and many other cases when people are afraid (I don't really know of what) to state their logic clearly.

share|improve this answer
    
Instead of ArgumentNullException, one should use code contracts, (unless if it's a pre-2010 source code which doesn't support code contracts). –  MainMa May 6 '12 at 18:40
    
I agree with you. If the null is not expected it should not be handled in strange ways but only reported. –  Giorgio May 6 '12 at 18:56
4  
if I read the question right, throwing an ArgumentNullException counts as "handling" the null value: it prevents a later null reference exception. –  Michael Edenfield May 6 '12 at 21:19
    
@MichaelEdenfield: I don't think that counts as real handling according to the given question (our goal is not to make your code work no matter what). To be honest, I often forget to write appropriate block that will throw if null is passed. However, I consider throwing NullArgumentException to be the best practice and the worst practice in my opinion is writing code that cannot handle null but instead of throwing exception returns e.g. another null as a method result (or empty string, or empty collection, or -1, or skips method execution if return type is void, etc.). –  empi May 6 '12 at 21:53
1  
@Giorgio, ArgumentNullException makes it very clear what the problem is and how to fix it. C# doesn't tend to use "undefined". If you call something in a way that's undefined, then the way you're calling it doesn't make sense. An exception is the proper way to indicate that. Now, sometimes I'll make parsing methods that return null if the int (for example) can't be parsed. But that's a case where an unparseable result is a legitimate possibility rather than a usage error. See also this article. –  Kyralessa May 7 '12 at 13:51

The habit of checking for null to my experience comes from former C or C++ developers, in those languages you have a good chance of hiding a severe error when not checking for NULL. As you know, in Java or C# things are different, using a null ref without checking for null will cause an exception, thus the error will not be secretly hidden as long as you don't ignore it on the calling side. So in most cases, checking explicitly for null does not make much sense and overcomplicates the code more than necessary. That's why I don't consider null checking to be a good habit in those languages (at least, not in general).

Of course, there are exceptions from that rule, here are those I can think of:

  • you want a better, human-readable error message, telling the user in what part of the program the wrong null reference occured. So your test for null throws just a different exception, with a different error text. Obviously, no error will get masked that way.

  • you want to make your program "fail more early". For example, you want to check for a null value of a constructor parameter, where the object reference otherwise would only be stored in a member variable and used later.

  • you can deal with the situation of a null ref safely, without the risk of subsequent faults, and without masking a severe error.

  • you want a completely different kind of error signaling (for example, returning an error code) in your current context

share|improve this answer
1  
"you want a better, human-readable error message" - that's the best reason to do it, in my opinion. "Object reference not set to an instance of an object" or "Null reference exception" is never as helpful as "Product not instantiated". –  pdr May 6 '12 at 16:31
    
@pdr: Another reason to check is to make sure it is always detected. –  Giorgio May 6 '12 at 18:57
9  
"Former C developers", perhaps. "Former C++ developers" only if those themselves are recovering C developers. As a modern C++ developer, I would be very suspicious about code that uses naked pointers or reckless dynamic allocation. In fact, I'd be tempted to turn the argument around and say that C++ does not suffer the problem that the OP describes because its variables don't have the additional special null-ness property that Java and C# have. –  Kerrek SB May 6 '12 at 21:47
4  
Your first paragraph is only half of the story: yes, if you use a null reference in C#, you will get an exception, whereas in C++ you will get undefined behavior. But in well-written C#, you are stuck with far more nullable references than in well-written C++. –  James McNellis May 6 '12 at 22:33

Use asserts to test and indicate pre/postconditions and invariants for your code.

It makes it so much easier to understand what the code expects and what it can be expected to handle.

An assert, IMO, is a suitable check because it is:

  • efficient (usually not used in release) ,
  • brief (usually a single line) and
  • clear (precondition violated, this is a programming error).

I think self-documenting code is important, therefore having a check is good. Defensive, fail-fast, programming makes maintenance way easier, which is where usually most time is spent.

Update

Wrt your elaboration. It's usually good to give the end user an application that works well under bugs, i.e. shows as much as possible. Robustness is good, cause heaven only knows what will break at a critical moment.

However , developers & testers must be aware of bugs ASAP, so you probably want a logging framework with a hook that can display an alert to the user that there was a problem. The alert should probably be shown to all users, but can probably be tailored differently depending on the runtime environment.

share|improve this answer

Fail early, fail often. null is one of the best unexpected values you can get, because you can fail quickly as soon as you try to use it. Other unexpected values are not so easy to detect. Since null automatically fails for you whenever you try to use it, I would say that it does not require an explicit check.

share|improve this answer
19  
I hope you meant: Fail early, fail fast, not often... –  empi May 6 '12 at 16:56
7  
The problem is that without explicit checks, a null value can sometimes propagate quite far before it causes an exception, and then it can be very hard to find out where it originated. –  Michael Borgwardt May 6 '12 at 16:57
    
@MichaelBorgwardt Isn't that what stack traces are for? –  R0MANARMY May 8 '12 at 20:28
2  
@R0MANARMY: stack traces don't help when a null value gets saved to some field and the NullPointerException is thrown much later. –  Michael Borgwardt May 8 '12 at 20:30

I would consider the question from the point of view of semantics, i.e. ask myself what a NULL pointer or a null reference argument represents.

If a function or method foo() has an argument x of type T, x can be either mandatory or optional.

In C++ you can pass a mandatory argument as

  • A value, e.g. void foo(T x)
  • A reference, e.g. void foo(T& x).

In both cases you do not have the problem of checking a NULL pointer. So, in many cases, you do not need a null pointer check at all for mandatory arguments.

If you are passing an optional argument, you can use a pointer and use the NULL value to indicate that no value was given:

void foo(T* x)

An alternative is to use a smart pointer like

void foo(shared_ptr<T> x)

In this case, you probably want to check the pointer in the code, because it makes a difference for method foo() whether x contains a value or no value at all.

The third and last case is that you use a pointer for a mandatory argument. In this case, calling foo(x) with x == NULL is an error and you have to decide how to handle this (the same as with an out-of-bounds index or any invalid input):

  1. No check: if there is a bug let it crash and hope this bug appears early enough (during testing). If it doesn't (if you fail to fail early enough), hope that it won't appear in a production system because the user experience will be that they see the application crash.
  2. Check all arguments at the beginning of the method and report invalid NULL arguments, e.g. throw an exception from foo() and let some other method handle it. Probably the best thing to do is to show the user a dialog window saying that the application has encountered an internal error and is going to be closed.

Apart from a nicer way of failing (giving the user more information), an advantage of the second approach is that it is more likely that the bug will be found: the program will fail every time the method is called with the wrong arguments whereas with approach 1 the bug will only appear if a statement dereferencing the pointer is executed (e.g. if the right branch of an if statement is entered). So approach 2 offers a stronger form of "fail early".

In Java you have fewer choices because all objects are passed using references which can always be null. Again, if the null represents a NONE value of an optional argument, then checking the null will (probably) be part of the implementation logic.

If the null is an invalid input, then you have an error and I would apply similar considerations as for the case of C++ pointers. Since in Java you can catch null pointer exceptions, approach 1 above is equivalent to approach 2 if method foo() dereferences all input arguments in all execution paths (which probably occurs very often for small methods).

Summarizing

  • Both in C++ and Java, checking if optional arguments are null is part of the program's logic.
  • In C++, I would always check mandatory arguments to ensure that a proper exception is thrown so that the program can handle it in a robust way.
  • In Java (or C#), I would check mandatory arguments if there are execution paths that won't throw in order to have a stronger form of "fail early".

EDIT

Thanks to Stilgar for more details.

In your case it seems that you have null as the result of a method. Again, I think you should first clarify (and fix) the semantics of your methods before you can take a decision.

So, you have method m1() calling method m2() and m2() returns a reference (in Java) or a pointer (in C++) to some object.

What is the semantics of m2()? Should m2() always return a non-null result? If this is the case, then a null result is an internal error (in m2()). If you check the return value in m1() you can have a more robust error handling. If you don't, you will probably have an exception, sooner or later. Maybe not. Hope that the exception is thrown during testing and not after deployment. Question: if m2() should never return null, why is it returning null? Maybe it should throw an exception instead? m2() is probably buggy.

The alternative is that returning null is part of the semantics of method m2(), i.e. it has an optional result, which should be documented in the the Javadoc documentation of the method. In this case it is OK to have a null value. Method m1() should check it as any other value: there is no error here but probably checking if the result is null is just part of the program logic.

share|improve this answer
    
In out case it was not the argument that was null. We made a database call for something that was expected to be present but in practice it was not present. The question is if we should check that every object exists or we should check only if we expect it to be missing. –  Stilgar May 6 '12 at 18:05
    
So the result returned by a method was a reference to an object and null was the value used to represent the result: NOT FOUND. Do I understand correctly? –  Giorgio May 6 '12 at 18:07
    
I have updated the question with additional details. –  Stilgar May 6 '12 at 18:26
  • Checking for a null should be your second nature

  • Your API will be used by other people and their expectation is likely to be different from yours

  • Thorough unit tests normally highlight a need for a null reference check

  • Checking for nulls doesn't imply conditional statements. If you worry that null checking makes your code less readable, then you may want to consider .NET code contracts.

  • Consider installing ReSharper (or similar) to search your code for missing null reference checks

share|improve this answer
    
Using code contracts for preconditions is simply glorified conditional statements. –  Michael Kjörling May 7 '12 at 11:28
    
Yes, I agree, but I also think that code looks cleaner when using code contracts, i.e. its readability is improved. –  CodeART May 7 '12 at 11:30

My general approach is to never test for an error condition that you do not know how to handle. Then the question that needs to be answered in each specific instance becomes: can you do something reasonable in the presence of the unexpected null value?

If you can continue safely by substituting another value (an empty collection, say, or a default value or instance), then by all means do so. @Shahbaz's example from World of Warcraft of substituting one image for another falls into that category; the image itself is likely just decoration, and has no functional impact. (In a debug build, I'd use a screaming color to draw attention to the fact that substitution has occured, but that depends greatly upon the nature of the application.) Log details about the error, though, so you know it's occuring; otherwise, you'll be hiding an error that you probably want to know about. Hiding it from the user is one thing (again, assuming processing can continue safely); hiding it from the developer is quite another.

If you cannot continue safely in the presence of a null value, then fail with a sensible error; in general, I prefer to fail as soon as possible when it's obvious that the operation cannot succeed, which implies checking preconditions. There are times when you don't want to fail immediately, but defer failure for as long as possible; this consideration comes up for example in cryptography-related applications, where side-channel attacks might otherwise divulge information you do not want known. At the end, let the error bubble up to some general error handler, which in turn logs relevant details and displays a nice (however nice they can be) error message to the user.

It's also worth noting that the proper response may very well differ between testing/QA/debug builds, and production/release builds. In debug builds, I would go for failing early and hard with very detailed error messages, to make it completely obvious that there is a problem and allow a post-mortem to determine what needs to be done to fix it. Production builds, when faced with safely recoverable errors, should probably favor recovery.

share|improve this answer
    
Of course there is general error handler up the chain. The problem with logs is that there may be no one to check them for very long time. –  Stilgar May 7 '12 at 16:45
    
@Stilgar, if there is no one to check the logs, then that is not a problem with the existence or absence of null checks. –  Michael Kjörling May 8 '12 at 7:35
1  
There is a problem. The end users may no notice that the system does not work correctly for a long time if there are null checks that simply hide the error and write it to the logs. –  Stilgar May 8 '12 at 7:42
    
@Stilgar, that depends entirely on the nature of the error. As I said in the post, only if you can continue safely should the unexpected null be replaced/ignored. Using one image instead of another in a release build (in debug builds, fail as early and as hard as possible so the problem becomes obvious) is a very different beast than returning invalid results for a calculation, for example. (Edited answer to include that.) –  Michael Kjörling May 8 '12 at 7:50
    
I'd go with failing fast and early even in production unless you can somehow log and report the error (without relying too much on savvy users). Hiding your deployed bugs from users is one thing - hiding them from yourself is dangerous. Also allowing the user to live with subtle bugs can erode their confidence in your application. Sometimes it is better to bite the bullet and let them know there was a bug and that you fixed it quickly. –  Merlyn Morgan-Graham May 21 '12 at 9:24

Sure NULL is not expected, but there are always bugs!

I believe you should check for NULL whether or not you don't expect it. Let me give you examples (although they are not in Java).

  • In World of Warcraft, due to a bug the image of one of the developers appeared on a cube for many objects. Imagine if the graphics engine didn't expect NULL pointers, there would have been a crash, while it was turned into a not so fatal (even funny) experience for the user. The very least is, you wouldn't have unexpectedly lost your connection or any of your save points.

    This is an example where checking for NULL prevented a crash and the case was silently handled.

  • Imagine a bank teller program. The interface does some checks and sends input data to an underlying part to perform the money transfers. If the core part expects not to receive NULL, then a bug in the interface can cause a problem in the transaction, possibly some money in the middle getting lost (or worse, duplicated).

    By "a problem" I mean a crash (as in crash crash (say the program is written in C++), not a null pointer exception). In this case, the transaction needs to roll back if NULL is encountered (which of course would not be possible if you didn't check the variables against NULL!)

I'm sure you get the idea.

Checking validity of arguments to your functions doesn't clutter up your code, as that is a couple checks at the top of your function (not spread in the middle) and greatly increases robustness.

If you have to choose between "the program tells the user that it has failed and gracefully shuts down or roll back to a consistent state possibly creating an error log" and "Access Violation", wouldn't you choose the first one? That means every function should be prepared for being called by a buggy code rather than expecting everything is correct, simply because bugs always exist.

share|improve this answer
    
Your second example disproves your point. In a bank transaction if anything goes wrong the transaction should abort. It is precisely when you cover the error that money can be lost or duplicated. Checking for null would be like "The customer sends null money... well I'll just send $10 (the equivalent of the image of the developer)" –  Stilgar May 7 '12 at 6:23
    
@Stilgar, The contrary! Checking for NULL would be abort with message. Not checking for NULL would be crash. Now crash in the beginning of transaction may not be bad, but in the middle can be catastrophic. (I didn't say the bank transfer should handle the error like the game! Just the fact that it should handle it) –  Shahbaz May 7 '12 at 8:38
2  
The whole point of a "transaction" is that it can't be half completed :) If there is an error then it is rolled back. –  Stilgar May 7 '12 at 16:33
    
This is terrible advice for anything where transactional semantics are actually required. You should make all your transactions be atomic and idempotent, otherwise any error you have will flat out guarantee lost or duplicated resources (money). If you have to deal with non-atomic or non-idempotent operations, you should wrap it very carefully in layers that guarantee atomic and idempotent behavior (even if you have to write those layers yourself). Always think: what happens if a meteor hits the web server at the moment this transaction is going on? –  Merlyn Morgan-Graham May 21 '12 at 9:11
1  
@MerlynMorgan-Graham, I did so. Hope that clears up the confusion. –  Shahbaz May 22 '12 at 9:33

Every null in your system is a ticking time bomb waiting to be discovered. Check for null at the boundaries where your code interacts with code you don't control, and forbid null within your own code. This rids you of the problem without naively trusting foreign code. Use exceptions or some sort of Maybe/Nothing type instead.

share|improve this answer

Since Fail early, fail fast as stated by @DeadMG (@empi) is not pssible because the web-application must work well under bugs you should enforce the rule

no exception catching without logging

so you as developpers are aware of potential problems by inspecting the logs.

if you are using a logging framework like log4net or log4j you can setup an additional special logging output (aka appender) that mail you errors and fatalerrors. so you stay informed.

[update]

if the enduser of the page should not be aware of the error you can setup an appender that mail you errors and fatalerrors you stay informed.

if it is ok that the enduser of the page sees cryptic errormessages you can setup an appender that puts the errorlog for the pageprocessing at the end of the page.

No matter how you use the logging if you swallow the exception the programm continue with data that makes sense and the swallowing is no more silent.

share|improve this answer
1  
The question is what happens to the page? –  Stilgar May 7 '12 at 7:45
    
How do we know that the data makes sense and the system is in a consistent state. After all the error was unexpected so we don't know what the impact was. –  Stilgar May 7 '12 at 16:37
    
"Since Fail early, fail fast as stated by @DeadMG (@empi) is not pssible because the web-application must work well under bugs you should enforce the rule": One could always catch all exceptions (catch (Throwable t)) and redirect to some error page. Wouldn't this be possible? –  Giorgio May 19 '12 at 5:58

While a null pointer exception will eventually be thrown it will often be thrown far from the point where you got the null pointer. Do yourself a favor and shorten the time it takes to fix the bug by at least logging the fact that you git a null pointer as soon as possible.

Even if you don't know what to do now, logging the event will save you time later. And maybe the guy who gets the ticket will be able to use the time saved to figure out the proper response then.

share|improve this answer

There are already good answers to this question, may be one addition to them: I do prefer assertions in my code. If your code can work only with valid objects, but nut with null, you should assert on null value.

Assertions in this kind of situation would let any unit and/or integration tests fail with the exact message, so you can nail down the problem pretty fast prior to production. If such an error slips through the tests and goes to production (where assertions should be deactivated) you gonna receive NpEx, and a severe bug, but it is better, than some minor bug which is much more fuzzy and popping up somewhere else in the code, and would be more expensive to fix.

Further more if someone must work with your code assertions tells him about your assumptions you made during designing/writing your code (in this case: Hey dude, this object must be presented!), which makes it easier to maintain.

share|improve this answer
    
Could you please write something to down-vote? I would appreciate a discussion. Thanx –  GeT May 11 '12 at 20:39
    
I didn't down vote, and I agree with your general idea. The language needs some work though. Your assertions define the contract of your API, and you fail early/fail fast if those contracts are violated. Doing this at the skin of your application will let users of your code know they did something wrong in their code. If they see a stack trace down in the recesses of your code, they will likely think your code is buggy - and you might think so too until you get a solid repro and debug it. –  Merlyn Morgan-Graham May 21 '12 at 9:18

I normally check for nulls but I "handle" unexpected nulls by throwing an exception that gives a bit more detail about exactly where the null occurred.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.