What’s your NullArgumentException?

26 02 2007

About a month ago Bill Venners asked, on Artima, What’s Your ShouldNeverHappenException?. Well, here’s an even more basic question: what’s your NullArgumentException? That is, what exception do you throw when a null value is passed for an argument that must not be null?

There are several obvious possibilities, all of which I’ve seen in real code at some point (often inconsistently):

  • Don’t worry about it, just leave the code to do whatever it happens to do, with the general expectation that this will probably result in a NullPointerException at some point (or even with the view that whether or not null is permitted is defined entirely by whether it happens to work or fails in some way).
  • Check the code to make sure it will fail with a NullPointerException due to dereferencing the argument, or due to passing it to another method that will fail etc. In the rare case where this won’t happen naturally, add in some additional code to make sure a NullPointerException gets thrown.
  • Explicitly check the argument at the start of the method and throw a NullPointerException if it’s null.
  • Explicitly check the argument at the start of the method and throw an IllegalArgumentException if it’s null.
  • Explicitly check the argument at the start of the method and throw some other standard JDK exception if it’s null (maybe depending on the precise nature of the method being called, the meaning of the argument, or whatever exception is thrown for other failures of the method).
  • Explicitly check the argument at the start of the method and throw some specifically-written exception if it’s null (that is, define your own exception type for this particular situation, or find a suitable third-party exception to use for it).

Personally, I tend to think that if an argument must be non-null then it’s best to explicitly validate this up-front at the start of the method. This gives you explicit control over the behaviour, independently of how the rest of the method is implemented, and it means you can safely assume a non-null value throughout the rest of the method. It also helps with the testing of the method, as the rejection of null can be tested on its own (knowing exactly what exception is expected), and the rest of the tests don’t have to worry about such nulls. It also ensures that the exception is thrown before anything is updated, so you avoid any complications from the method carrying out some updates but not others.

Actually, that’s my approach for all argument validation, not just checks for non-null: explicitly check the argument values at the start of the method, and explicitly throw the relevant exception for any invalid argument before proceeding with the actual processing of the method. Of course, in keeping with this the Javadoc needs to spell out any restrictions on the arguments and what happens for invalid values (e.g. what exception is thrown), and there should be test-cases to verify each such validation.

As for what exception should be thrown, I tend to think that an IllegalArgumentException is more appropriate than a NullPointerException. This is partly my interpretation of their somewhat-overlapping Javadoc definitions (“Thrown to indicate that a method has been passed an illegal or inappropriate argument” vs. “Thrown when an application attempts to use null in a case where an object is required” with examples that cover various types of dereferencing but do not mention passing as an argument). It also tends to be more consistent with any other argument validation. But mainly it’s so that any NullPointerException that my code does produce is immediately known to be due to a “real” coding error rather than just from passing null to a method that is known to not allow it.

Having said that, I think that the “null argument” case is sufficiently common and significant that it warrants its own specific exception. So the exception that I throw for null argument values is my own custom subclass of IllegalArgumentException that I’ve called… you’ve guessed it… NullArgumentException. Instead of the usual constructors taking a message and/or cause, I’ve given this a single constructor that takes the name of the relevant argument as a string and produces a standardized message that shows that argument name as part of the message. This also helps standardize the testing of these argument validations (though the code for such tests still has slightly more “boilerplate” than I’d like).

So my code for such validation of non-null argument values always takes the following form:

/**
 * ... normal javadoc...
 *
 * @param foo    ...normal javadoc... Must be non-null.
 * @throws NullArgumentException if the given foo is null.
 */
public void doSomething(final SomeType foo) {
    if (foo == null) {
        throw new NullArgumentException("foo");
    }
    ...(process foo, knowing it is non-null)...
}

If the NullArgumentException is thrown, its message then says “Argument ‘foo’ is null but the method requires a non-null value” (and the point where the exception was thrown shows the method involved). At the same time it’s still an IllegalArgumentException (being a subclass of it).

Since adopting this approach, with my own NullArgumentException class, I’ve discovered that the Apache Jakarta project’s Jakarta Commons subproject provides exactly such a NullArgumentException in its Commons Lang library. So I’m guilty of re-inventing the wheel, though I don’t otherwise use the Jakarta Commons libraries so on the whole I’m happy to stick with my own exception class and avoid introducing any dependency on Jakarta Commons just for this.

This approach has worked well for me so far, to the extent that I’ve taken the same approach for various other common argument validations (for example, where an array or object argument is non-null but contains a null for some particular element/property that ought to be non-null, and where a string argument must be a non-empty string).

But I’d be interested in what everyone else does for this, especially whether there’s any consensus on whether null arguments should be reported by a NullPointerException or an IllegalArgumentException, or any further twists that anyone has on this subject.

So what’s your “NullArgumentException”?

Advertisements

Actions

Information

15 responses

27 02 2007
Laird Nelson

I’ll bite. I tend to try wherever possible to make sure that my method does something correct with null inputs. So I don’t tend to fit one of your styles above. For methods that can return something, I try to make it the case that if they get nulls in, they return null. For methods that cannot return something, then I make it the case that they do nothing.

This flies in the face of arguments I actually used to subscribe to, which go something like this: look, if someone passed you something that you weren’t expecting, you should fail as quickly as you can, rather than risk hiding the problem. But I think that in lots of cases you can get around the very good point raised by this argument by simply recasting what a valid input is.

What led me to this point of view was having to debug applications that stay alive for days or weeks at a time without shutdown or reboot and discovering lots of well-meaning RuntimeExceptions being thrown–and wreaking havoc on the rest of the app. It made me think that in terms of robustness–here defined as the ability to soldier on even with a mortal injury–you’re better off making sure your methods can be called with all sorts of inputs, including nulls, because you simply can’t predict what the maintenance programmer upstream from you did to violate your previously inviolable contract.

It’s late; I hope that made sense.

Cheers,
Laird

27 02 2007
closingbraces

Thanks Laird, that’s interesting and does make sense.

I guess it’s an even more fundamental question of each method’s design – what range of inputs are catered for, and whether you design this to be as broad as possible or as narrow as possible. It’s probably also relevant whether the system is entirely under your own control or includes third-party code calling your methods, how well written/documented/tested each method is, and how good callers are at obeying the contract. Some people also use the “Null Object” pattern to try to avoid having nulls altogether.

Presumably in a “stay alive” application you have to catch and report/log any RuntimeExceptions that occur, treat the relevant request or transaction as failed, and then carry on processing? I can see it being a pain to debug lots of unnecessary exceptions all over the place. Though personally I’d still rather have that and fix the individual bugs and misunderstandings rather than avoiding the failure (with perhaps somewhat unknown overall consequences) – but that’s easy for me to say!

I have seen one high-volume system where all the failures where not only logged but also automatically merged/summarized into a bug database. Someone on the team was tasked with monitoring this and gradually worked through the bugs, prioritized by frequency. They found and fixed a lot of quite subtle bugs that were previously just being ignored as “non-critical”, and got the remaining failures down to a very low level.

But certainly there are places where null is a perfectly reasonable value for an argument and for which appropriate behaviour can be defined.

Cheers,
Mike

27 02 2007
Ricky Clarkson

Because I don’t usually write APIs, but code that is all part of one project, I just set a rule – null is not allowed. I believe Eclipse uses that rule.

That way I don’t have to write code that checks for null, except when I’m calling existing APIs that may return null.

27 02 2007
Carfield Yim

Sound like a lot of duplicating checking, except if you use AOP, isn’t it?

27 02 2007
B. K. Oxley (binkley)

My standard null argument check method is in a Contract utility class:

public static T asNotNull(final T input, final String message) {
if (null == input)
throw new IllegalArgumentException(message);

return input;
}

27 02 2007
B. K. Oxley (binkley)

I see that the comment software stripped the angle brackets from my pasted code example. The asNotNull method is generic with “T” is the type parameter.

The idea is to return the input as long as it is non-null. This lends itself to uses such as:

SomeClass(SomeOtherType pointer) {
this.pointer = Contract.asNotNull(pointer, “Missing pointer”);
}

Which I find easier to read and use than explicit test-and-throw blocks.

27 02 2007
closingbraces

Thanks Ricky, Carfield, B.K,

Yes, there are duplicated checks, in the sense that the specific method arguments for which null isn’t acceptable all need an “if null, throw exception” (or something like B.K’s “Contract.asNull” call). But it’s miniscule and trivial in the overall sweep of things. I doubt AOP would help, as you’d still need to determine exactly which arguments need the check, and having a central list of them somewhere seems at least as bad as maintaining them “in line”.

JSR 305 “@NonNull” annotations (plus JSR 308) might help, though I suspect I’d still want to retain the actual checks unless and until the compiler or JVM enforce them (rather than the annotations just being information for other tools to optionally analyze).

In a sense, for code that isn’t a public API called by other people’s code, the checks are only relevant during testing, because if my tests are sufficient and don’t raise these exceptions then I can be confident that none of my own code violates these checks… and in theory they therefore aren’t necessary. Possibly that’s an argument for using an “assert” instead – I forgot “assert” in the original list of possiblities. But for the minimal effort involved I prefer to have a “real” check and a very clear and explicit failure if anything goes wrong, and in any case pretty much all of my code is intended for other people to call (at least potentially).

I quite like the “Contract.asNotNull” approach… I originally decided against anything like that so that the exception stack trace would alway show the exception as coming from the specific method involved rather than from a centralized check. But I might reconsider that trade-off – and in any case the check could always examine the stack trace and include the relevant method details in the exception message itself.

28 02 2007
arsenalist

How about assert statements? If you ever dig down the Spring framework’s code, they make excellent use of them to great effect in exactly these situations.

28 02 2007
closingbraces

Ah yes, assertions – some love ’em, some hate ’em.

I liked the idea at first, was glad to see them introduced, and started using them for a few internal consistency checks. But over time I found that most places I’d consider an assertion, I was better off having a “real” check and exception, or the relevant condition was so clearly impossible that the assertion looked silly and seemed a waste of time (and untestable, and skewing code-coverage results). Gradually used them less and less, and eventually ripped out the few remaining assertions and replaced them with real checks. In the end I’ve found so few situations where they seem to be the right answer, that it seems better to not use them at all and have one less thing to think about. That’s just my personal view. Your mileage may vary, of course.

For this particular scenario, asserts might be a decent answer if all the calls are within your own code (e.g. all one project etc) and you regard these as internal consistency checks, to be used during testing, not needed otherwise, and not really needed for correct operation of the code. But even then there doesn’t seem much lost by doing a non-assertion check – maybe a few more characters of source code, but logically no more complex. If, instead, it’s code that can be called from elsewhere (even potentially), then I think you need proper validation rather than assertions (which can be, and normally would be, turned off in real use – and arguably don’t result in the appropriate type of exception). More generally, assertions are very clearly not intended for validating pulic method arguments (e.g. see http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html#usage “Do not use assertions for argument checking in public methods…”).

p.s. On that “other” subject, I thought the gunners kids were magnificent sunday, apart from the end. They’ll learn though.

28 02 2007
Tom Klaasen

If it makes sense to handle null, or if it doesn’t make sense but there’s a logical answer for null, then I’ll accept the value and treat it. More often than not, this just returns null again. This is then also documented explicitly in the javadoc.
For the cases where I want to be sure that the argument is not null, I use Spring’s Assert class — not the JDK1.4 assertions, because they can be shut off. It reads like ‘Assert.notNull(arg);’. It resembles the Contract solution, but it reuses common library code, a tactic of which I’m a huge fan.

My 2 cents.

28 02 2007
closingbraces

Thanks Tom, so that’s another alternative to Commons Lang or roll-your-own.

I’d note that Spring’s Javadoc for this says “Mainly for internal use within the framework; consider Jakarta’s Commons Lang >= 2.0 for a more comprehensive suite of assertion utilities”. You also need to give the whole exception message that you want, though there’s also an overload with no message argument that presumably results in a default messsage which won’t show the relevant argument name. So you might need your own rules and discipline to give suitable messages (as per, for example, http://tp.its.yale.edu/pipermail/cas/2006-February/002298.html).

I’m not sure what Commons Lang “assertion facilities” the javadoc is referring to – there’s a Validator class that’s very similar in approach to Spring’s Assert method, but doesn’t seem any more comprehensive.

28 02 2007
closingbraces

To summarize, so far it looks like:

– It seems fairly well established that (where an exception is to be thrown rather than handling the null as a valid value) the exception should be an IllegalArgumentException rather than NullPointerException or anything else.

– One option is to use a specialized subclass of IllegalArgumentException, so as to simplify its construction, standardize the message etc – with Commons Lang’s NullArgumentException being an example of this.

– Options for carrying out the check and throwing the exception include simply doing it yourself, or calling your own centralized method to do it, or using a third-party library such as Commons Lang’s “Validator” class or Spring’s “Assert” class (there are probably many more). Usual pros/cons re writing it yourself or using some particular third-party library, of course.

2 03 2007
William Woody

I’ll bite.

Me; if the function call is cheap–that is, if adding a check for a null reference would significantly add to the function–then I just let it die with a NullPointerException.

If the function call is expensive or has some side effects which cannot be easily unrolled–then I ‘preflight’ all of the arguments and throw IllegalArgumentException. (And I generally throw IllegalArgumentException rather than NullPointerException because while the illegal argument is illegal because it’s null, I always saw NullPointerException as a complaint by the Java VM that it is trying to dereference null.)

2 03 2007
closingbraces

I’ve just seen an example of why arguments should usually be validated “up front” on Brian Duff’s blog.

As Brian Duff points out, this is also discussed in item 23 of Joshua Bloch’s Effective Java book. Re-reading that, one interesting point (and the book is full of them) is that not validating arguments can be a problem if their values are stored and not actually used until later.

I’d also note that item 42 in that book says that “convention dictates” that NullPointerException should be thrown for invalidly-null arguments instead of IllegalArgumentException. So I guess Joshua Bloch’s answer to this question would probably be “NullPointerException”, at least at the time of writing of the book – though possibly only due to everyone else doing it rather than it actually being the ideal.

6 03 2007
Chrstian Möller

Hi,

Jakarta Commons’ solution NullArgumentException and Validate has been mentioned before. The only question arising to me is this: Why do they not throw NullArgumentException with Validate.notNull(...) but IllegalArgumentException?

OK, seems that this is a little bit out of scope here; I should post it on Jakarta’s list, shouldn’t I? 🙂

Christian

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s




%d bloggers like this: