views:

3892

answers:

19

I have a simple setter method for a Java property and null is not appropriate for this particular property. I have always been torn, in this situation: should I throw an IllegalArgumentException, or a NullPointerException? From the javadocs, both seem appropriate. Is there some kind of understood standard? Or is this just one of those things that you should do whatever you prefer and both is really correct?

+25  A: 

I'm not a Java developer, but just from the sound of it, it seems like an IllegalArgumentException is called for if you don't want null to be an allowed value, and the NullPointerException would be thrown if you were trying to use a variable that turns out to be null.

Greg Hurlman
+4  A: 

If it's a setter method and null is being passed to it, I think it would make more sense to throw an IllegalArgumentException. A NullPointerException seems to make more sense in the case where you're attempting to actually use the null.

So, if you're using it and it's null, NullPointer. If it's being passed in and it's null, IllegalArgument.

Jeremy Privett
A: 

I think you should definitely throw a IllegalArgumentException and thus fail-fast. Let other developers know by marking it in the JavaDocs and also define constraints on your methods, so that they see what happens when they pass an invalid objects. I wrote about this a couple of weeks ago, if you want to follow up.

david
+12  A: 

I tend to follow the design of JDK libraries, especially Collections and Concurrency (Joshua Bloch, Doug Lea, those guys know how to design solid APIs). Anyway, many APIs in the JDK pro-actively throws NullPointerException.

For example, the Javadoc for Map.containsKey states:

@throws NullPointerException if the key is null and this map does not permit null keys (optional).

It's perfectly valid to throw your own NPE. The convention is to include the parameter name which was null in the message of the exception.

The pattern goes:

public void someMethod(Object aParam) {  
if (mustNotBeNull == null) {
throw new NullPointerException("aParam");

}
}

Whatever you do, don't allow a bad value to get set and throw an exception later when other code attempts to use it. That makes debugging a nightmare. You should always the follow the "fail-fast" principle.

Mark Renouf
A: 

If you choose to throw a NPE and you are using the argument in your method, it might be redundant and expensive to explicitly check for a null. I think the VM already does that for you.

jassuncao
+1  A: 

The accepted practice if to use the IllegalArgumentException( String message ) to declare a parameter to be invalid and give as much detail as possible... So to say that a parameters was found to be null while exception non-null, you would do something like this:

if( variable == null )
    throw new IllegalArgumentException("The object 'variable' cannot be null");

You have virtually no reason to implicitly use the "NullPointerException". The NullPointerException is an exception thrown by the Java Virtual Machine when you try to execute code on null reference (Like toString()).

Shadow_x99
+25  A: 

The standard is to throw the NullPointerException. The generally infallible "Effective Java" discusses this briefly in Item 42 (in the first edition) "Favor the use of standard exceptions":

"Arguably, all erroneous method invocations boil down to an illegal argument or illegal state, but other exceptions are standardly used for certain kinds of illegal arguments and states. If a caller passes null in some parameter for which null values are prohibited, convention dictates that NullPointerException be thrown rather than IllegalArgumentException."

GaryF
I strongly disagree. NullPointerExceptions should only be thrown if the JVM accidentially follows a null reference. That is the difference that helps you when called in to look at code at 3 in the morning.
Thorbjørn Ravn Andersen
I don't necessarily agree with the standard (I could actually go either way on the issue), but that IS what the standard usage throughout the JDK is, hence Effective Java making the case for it. I think this is a case of choosing whether or not to follow the standard, or do the thing you feel is right. Unless you have a very good reason (and this certainly may qualify), it's best to follow standard practice.
GaryF
Actually, the NullPointerException is supposed to be thrown for "illegal uses of the null reference". Indeed, encapsulation means you should *know* whether it was caused by the JVM actually dereferencing a bad pointer or the app just rejecting it.
Christopher Smith
A: 

The definitions from the links to the two exceptions above are IllegalArgumentException: Thrown to indicate that a method has been passed an illegal or inappropriate argument. NullPointerException: Thrown when an application attempts to use null in a case where an object is required.

The big difference here is the IllegalArgumentException is supposed to be used when checking that an argument to a method is valid. NullPointerException is supposed to be used whenever an object being "used" when it is null.

I hope that helps put the two in perspective.

martinatime
The salient bit is that it is the *application* that is using null, not the runtime. So there is a fairly large overlap between "when a method has been passed an illegal or inappropriate argument" and "when an application is using null". In theory, if an app passes a null for a field that requires non-null, both criteria are being met.
Christopher Smith
+1  A: 

Couldn't agree more with what's being said. Fail early, fail fast. Pretty good Exception mantra.

The question about which Exception to throw is mostly a matter of personal taste. In my mind IllegalArgumentException seems more specific than using a NPE since it's telling me that the problem was with an argument I passed to the method and not with a value that may have been generated while performing the method.

My 2 Cents

Allain Lalonde
A: 

If it's a "setter", or somewhere I'm getting a member to use later, I tend to use IllegalArgumentException.

If it's something I'm going to use (dereference) right now in the method, I throw a NullPointerException proactively. I like this better than letting the runtime do it, because I can provide a helpful message (seems like the runtime could do this too, but that's a rant for another day).

If I'm overriding a method, I use whatever the overridden method uses.

erickson
A: 

IllegalArgumentException makes more sense, since it clearly shows that the problem was an invalid argument passed to the method.

+5  A: 

In general, a developer should never throw a NullPointerException. This exception is thrown by the runtime when code attempts to dereference a variable who's value is null. Therefore, if your method wants to explicitly disallow null, as opposed to just happening to have a null value raise a NullPointerException, you should throw an IllegalArgumentException.

+36  A: 

You should be using IllegalArgumentException (IAE), not NullPointerException (NPE) for the following reasons:

First, the NPE JavaDoc explicitly lists the cases where NPE is appropriate. Notice that all of them are thrown by the runtime when null is used inappropriately. In contrast, the IAE JavaDoc couldn't be more clear: "Thrown to indicate that a method has been passed an illegal or inappropriate argument." Yup, that's you!

Second, when you see an NPE in a stack trace, what do you assume? Probably that someone dereferenced a null. When you see IAE, you assume the caller of the method at the top of the stack passed in an illegal value. Again, the latter assumption is true, the former is misleading.

Third, since IAE is clearly designed for validating parameters, you have to assume it as the default choice of exception, so why would you choose NPE instead? Certainly not for different behavior -- do you really expect calling code to catch NPE's separately from IAE and do something different as a result? Are you trying to communicate a more specific error message? But you can do that in the exception message text anyway, as you should for all other incorrect parameters.

Fourth, all other incorrect parameter data will be IAE, so why not be consistent? Why is it that an illegal null is so special that it deserves a separate exception from all other types of illegal arguments?

Finally, I accept the argument given by other answers that parts of the Java API use NPE in this manner. However, the Java API is inconsistent with everything from exception types to naming conventions, so I think just blindly copying (your favorite part) of the Java API isn't a good enough argument to trump these other considerations.

Jason Cohen
+1 for excellent reasoning
Galghamon
A: 

You should throw an IllegalArgumentException, as it will make it obvious to the programmer that he has done something invalid. Developers are so used to seeing NPE thrown by the VM, that any programmer would not immediately realize his error, and would start looking around randomly, or worse, blame your code for being 'buggy'.

Will Sargent
Sorry, if a programmer looks around "randomly" upon getting any kind of exception... changing the name of an exception isn't going to help much.
Christopher Smith
+3  A: 

It's a "Holy War" style question. In others words, both alternatives are good, but people will have their preferences which they will defend to the death.

Steve McLeod
A: 

In this case, IllegalArgumentException conveys clear information to the user using your API that the " should not be null". As other forum users pointed out you could use NPE if you want to as long as you convey the right information to the user using your API.

GaryF and tweakt dropped "Effective Java" (which I swear by) references which recommends using NPE. And looking at how other good APIs are constructed is the best way to see how to construct your API.

Another good example is to look at the Spring APIs. For example, org.springframework.beans.BeanUtils.instantiateClass(Constructor ctor, Object[] args) has a Assert.notNull(ctor, "Constructor must not be null") line. org.springframework.util.Assert.notNull(Object object, String message) method checks to see if the argument (object) passed in is null and if it is it throws a new IllegalArgumentException(message) which is then caught in the org.springframework.beans.BeanUtils.instantiateClass(...) method.

+1  A: 

I wanted to single out Null arguments from other illegal arguments, so I derived an exception from IAE named NullArgumentException. Without even needing to read the exception message, I know that a null argument was passed into a method and by reading the message, I find out which argument was null. I still catch the NullArgumentException with an IAE handler, but in my logs is where I can see the difference quickly.

jkf
I have adopted the "throw new IllegalArgumentException("foo == null")" approach. You need to log the variable name anyway (to be certain that you are looking at the right sttatement etc)
Thorbjørn Ravn Andersen
+3  A: 

Apache Commons Lang has a NullArgumentException that does a number of the things discussed here: it extends IllegalArgumentException and its sole constructor takes the name of the argument which should have been non-null.

While I feel that throwing something like a NullArgumentException or IllegalArgumentException more accurately describes the exceptional circumstances, my colleagues and I have chosen to defer to Bloch's advice on the subject.

Brian T. Grant
+1  A: 

Voted you up Jason Cohen's argument because it was well presented. Let me dismember it step by step. ;-)

  • The NPE JavaDoc explicitly says "other illegal uses of the null object". If it was just limited to situations where the runtime encounters a null when it shouldn't, all such cases could be defined far more succinctly.

  • Can't help it if you assume the wrong thing, but assuming encapsulation is applied properly, you really shouldn't care or notice whether a null was dereferenced inappropriately vs. whether a method detected an inappropriate null and fired an exception off.

  • I'd choose NPE over IAE for multiple reasons

    • It is more specific about the nature of the illegal operation
    • Logic that mistakenly allows nulls tends to be very different from logic that mistakenly allows illegal values. For example, if I'm validating data entered by a user, if I get value that is unacceptable, that's PEBKAC. If I get a null, that's programmer error.
    • Invalid values can cause things like stack overflows, out of memory errors, parsing exceptions, etc. Indeed, most errors generally present, at some point, as an invalid value in some method call. For this reason I see IAE as actually the MOST GENERAL of all exceptions under RuntimeException.
  • Actually, other invalid arguments can result in all kinds of other exceptions. "UnknownHostException", "FileNotFoundException", a variety of syntax error exceptions, IndexOutOfBoundsException, authentication failures, etc., etc.

In general, I feel NPE is much maligned because traditionally has been associated with code that fails to follow the "fail fast" principle. That, plus the JDK's failure to populate NPE's with a message string really has created a strong negative sentiment that isn't well founded. Indeed, the difference between NPE and IAE from a runtime perspective is strictly the name. From that perspective, the more precise you are with the name, the more clarity you give to the caller.

Christopher Smith