4

So, I'm working on designing a class wherein if certain arguments to certain methods are null, either the method (or the object as a whole) won't work.

I know that it'll throw a NullPointerException once it receives the null object and attempts to use it, but I want the programmer trying to call the method to understand that the bug is not in my code. I just want to ensure that the resulting exception thrown would be very clear (without the need to look into my source).

I've seen a few examples of what I described, where they throw an IllegalArgumentException when the parameter is null.

Here's the difference, imagine that someObject will somehow be vital to the method:

public void doSomething(SomeClass someObject) {
    if (someObject == null) throw new IllegalArgumentException("someObject is null");
    ...
}

This way, the programmer understands that he or she has broken the contract implied by the javadoc (whether or not it is explicitly stated).

Is that good practice, or even a reasonable thing to do?

Quick Edit/Side-bar:

What would be best to say in the exception message?

Is it better to state what "went wrong":

someObject is null

Or is it better to state that something "went wrong" and generally imply the cause (and ultimately the solution):

someObject cannot be null

Or is there a better alternative?

Community
  • 1
  • 1
Bhaxy
  • 5,346
  • 10
  • 39
  • 41
  • 1
    possible duplicate of [IllegalArgumentException or NullPointerException for a null parameter?](http://stackoverflow.com/questions/3881/illegalargumentexception-or-nullpointerexception-for-a-null-parameter) – Anthony Pegram Feb 21 '12 at 04:37
  • 1
    @AnthonyPegram - I think this question is less about which of the two to choose than it is about whether the value should be checked at the start of the method or allow the NPE to be thrown when the variable is used. – derekerdmann Feb 21 '12 at 04:48
  • If you get to the point that it becomes too repetitive, don't forget that most good IDE's have some kind of code generation (e.g. Eclipse has "code templates" which you can use to create a "notnull" that automatically creates an IllegalArgumentException on null using the parameter name in the string). – Maarten Bodewes Feb 21 '12 at 19:01

5 Answers5

5

Use an IllegalArgumentException instead of allowing the NullPointerException so that you can discover the error as early as possible. You could perform the same check and throw a NullPointerException yourself as well, but that's a matter of semantics.

By throwing the error immediately, it helps you or some other developer catch the mistake before anything else happens. If your method doesn't use the argument immediately (for instance, in a setter), then the problem could appear to be the result of some completely different operation.

If you're not familiar with it, a fantastic book called The Pragmatic Programmer has a tip that I'm trying to emphasize here:

Time 32: Crash Early

The main idea is to fail before anything bad can happen, rather than allow problems to crop up at unexpected times.

This would also be a good place to use an assertion to enforce your precondition. Make sure you document that the argument cannot be null, and make your method look like this:

public void doSomething(SomeClass someObject) {
    assert someObject != null    
    ...
}

If the assertion fails, an error will be thrown. While this could be turned off with compiler options, it's a good practice during development.

derekerdmann
  • 17,696
  • 11
  • 76
  • 110
  • 2
    +1 for crash early. I don't think this is a good place for assertions though, since they can be disabled by whoever runs your code. Assertions are usually only desirable for private methods where the erroneous input would be coming from somewhere else in the class which should have already validated the input. In other words, if it's an exposed method and so the input might be coming from outside the "library", validate with a runtime exception. If it's internal and the input is coming from another place in the library, validate with an assertion. – Mark Peters Feb 21 '12 at 05:02
  • 1
    @MarkPeters - I guess I have to agree with you on the assertions. I suggested it since I assumed this was a small project, possibly for a class, and to introduce the concept of assertions, since they're very relevant to method preconditions. – derekerdmann Feb 21 '12 at 05:10
4

I would say the standard, documented practice is to just use NullPointerException.

  1. It is used explicitly in many @throws Javadocs to document the case when an argument is null and is not allowed to be (Reader.read(CharBuffer), String.contains(CharSequence) and many, many more)
  2. It is recommended in respected coding guidelines (for example, Bloch recommends its use in Effective Java).
  3. The class itself says that it's meant to be thrown explicitly:

    Applications should throw instances of this class to indicate other illegal uses of the null object.

  4. It's what Guava's Preconditions throw for checkNotNull, and they know Java library design.
  5. Using a NullPointerException doesn't prevent you from giving a good error message describing the exception. By doing so you would set such stacktraces apart from the default NPE stacktrace.
If you really want to deviate from the standard, you can use IllegalArgumentException but at that point I'd go one further and create a subclass called NullArgumentException.

Edit

Speaking of Preconditions, you seem on track to be using runtime exceptions effectively for checking values and invariants. I highly recommend you just use Guava's Preconditions library to standardize and simplify this. Your example would just boil down to:

public void doSomething(SomeClass someObject) {
    Preconditions.checkNotNull(someObject, "someObject cannot be null");
    //...
}
Mark Peters
  • 80,126
  • 17
  • 159
  • 190
1

You can use either. Just document your choice and be consistent. If clients are likely to use your library with another, being consistent with that will likely help clients as well.

Mike Samuel
  • 118,113
  • 30
  • 216
  • 245
1

Both approaches are reasonable. If your javadoc says that passing a null will result in a NPE, and the user of your API gets a NPE, then it sounds to me like the function is working as advertised!

I find that a NPE is easiest when the argument is to be used in that method. If you're going to be saving the reference for later (e.g., in a constructor), then the IllegalArgumentException makes a bit more sense. In other words, throw a NPE if that comes naturally, and throw an IAE if it saves a future NPE.

yshavit
  • 42,327
  • 7
  • 87
  • 124
  • +1 very sound advice - it's much harder to debug if you allow the null to get stored and cause problems at some indeterminate point in the future! – mikera Feb 21 '12 at 05:29
0

I prefer using NPE, but I would validate the input in the beginning of the method, because otherwise my object's state can be changed to something illegal (because of some non-complete work).

Amir Pashazadeh
  • 7,170
  • 3
  • 39
  • 69