7

Do you know some nice alternative to Apache Commons Validate or Guava Preconditions that would throw IllegalArgumentException instead of NullPointerException when checking if object is not null (except Spring Assert)?


I'm aware that Javadocs say:

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

Nevertheless, I just don't like it. For me NPE was always meaning I just forgot to secure null reference somewhere. My eyes are so trained, I could spot it browsing logs with a speed of few pages per second and if I do there is always bug alert in my head enabled. Therefore, it would be quite confusing for me to have it thrown where I expect an IllegalArgumentException.

Say I have a bean:

public class Person {
  private String name;
  private String phone;
  //....
}

and a service method:

public void call(Person person) {
  //assert person.getPhone() != null
  //....
}

In some context it may be ok, that a person has no phone (my grandma doesn't own any). But if you'd like to call such person, for me it's calling the call method with an IllegalArgument passed. Look at the hierarchy - NullPointerException is not even a subclass of IllegalArgumentException. It basically tells you - Again you tried to call a getter on null reference.

Besides, there were discussions already and there is this nice answer I fully support. So my question is just - do I need to do ugly things like this:

Validate.isTrue(person.getPhone() != null, "Can't call a person that hasn't got a phone");

to have it my way, or is there a library that would just throw IllegalArgumentException for a notNull check?

Community
  • 1
  • 1
makasprzak
  • 5,082
  • 3
  • 30
  • 49
  • 4
    Not clear on what you mean by "a correctly handled situation". Sure, it's not an accidental dereference of `null`, but it amounts to the same thing: a method is being called with `null` passed in for a parameter that cannot be `null`, which is every bit as much a programmer error. – ColinD May 04 '15 at 20:18
  • sure, but in this case NPE (in my interpretation) is handled, the IllegalArgumentException is not handled. And actually it usally is by some servlet level exception handler or smth. – makasprzak May 04 '15 at 20:21
  • 1
    FWIW, the JDK sets a pretty firm precedent that this is how *it* expects invalid nulls to be handled. – Louis Wasserman May 04 '15 at 20:23
  • Still I would probably like to find a library that is against those expectations:) – makasprzak May 04 '15 at 20:31
  • Unfortunately, it seems like Guava won't be changing the NPE to IAE anytime soon, [link](https://github.com/google/guava/issues?utf8=%E2%9C%93&q=checkNotNull). I feel you make a strong argument for IAE over NPE here. – Captain Man May 04 '15 at 20:35
  • @ColinD - I gave up and updated the question regarding your comment – makasprzak May 04 '15 at 20:36
  • 1
    "it would be quite confusing for me to have it thrown where I expect an IllegalArgumentException." -- And for the people maintaining your code, it will be quite confusing to receive an IAE instead of a NPE... Just use the standards. Even [`java.util.Objects.requireNotNull` & friends](http://docs.oracle.com/javase/8/docs/api/java/util/Objects.html#requireNonNull-T-) throw an NPE. – Olivier Grégoire May 04 '15 at 21:47

7 Answers7

8

Since the topic of this question evolved into "Correct usage of IllegalArgumentException and NullpointerException", I would like to point out the strait forward answer in Effective Java Item 60 (second edition):

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. Similarly, if a caller passes an out-ofrange value in a parameter representing an index into a sequence, IndexOutOfBoundsException should be thrown rather than IllegalArgumentException.

Fritz Duchardt
  • 11,026
  • 4
  • 41
  • 60
5

What about Preconditions's checkArgument?

public void call(Person person) {
    Preconditions.checkArgument(person.getPhone() != null);
    // cally things...
}

checkArgument throws IllegalArgumentException instead of NullPointerException.

Captain Man
  • 6,997
  • 6
  • 48
  • 74
  • yeah, apache-commons Validate.isTrue also throws IllegalArgumentException, but Validate.notNull throws NPE to my disappointment. – makasprzak May 04 '15 at 20:13
  • `Preconditions.checkArgument( ... != null);` may not be what your looking for, but it still reads a little better *(in my opinion)* than `Validate.isTrue( ... != null);` because it's clear it has to do with preconditions and arguments from the names. – Captain Man May 04 '15 at 20:39
  • 1
    well, yes that's probably better. +1 ;) – makasprzak May 04 '15 at 20:41
  • 2
    @macias `NullPointerException` is commonly recommended over `IllegalArgumentException` for null arguments (see _Effective Java_ Item 60 [as above](https://stackoverflow.com/a/30172390/27358)). It’s also fairly common in the standard libraries, e.g. in [URI.create()](https://docs.oracle.com/javase/7/docs/api/java/net/URI.html#create(java.lang.String)). It's the same logic as throwing `IndexOutOfBoundsException` for an out-of-bounds argument to [List.get()](https://docs.oracle.com/javase/7/docs/api/java/util/List.html#get(int)) -- more specific & the error tends to arise for specific reasons. – David Moles Nov 28 '17 at 23:55
  • 1
    @DavidMoles the problem to me is that `NullPointerException` is *not* more specific than `IllegalArgumentException`. To me, NPE means "something bad happened with a null" which seems less specific than "An invalid argument was passed". In addition, I think most people immediately think NPE is "a null pointer was dereferenced". – Captain Man Nov 29 '17 at 13:30
  • @CaptainMan use the more specific [Preconditions.checkNotNull(person.getPhone())](https://guava.dev/releases/23.0/api/docs/com/google/common/base/Preconditions.html#checkNotNull-T-) instead, it throws NPE. – Stefan L Jul 05 '19 at 14:02
  • @StefanL Please re-read the question. "alternative to [...] Guava Preconditions that would **throw IllegalArgumentException instead of NullPointerException** when checking if object is not null?" `Preconditions#checkNotNull` throws `NullPointerException` when the argument is null, not `IllegalArgumentException`. – Captain Man Jul 08 '19 at 14:22
1

You can use valid4j with hamcrest-matchers (found on Maven Central as org.valid4j:valid4j). The 'Validation' class has support for regular input validation (i.e. throwing recoverable exceptions):

import static org.valid4j.Validation.*;

validate(argument, isValid(), otherwiseThrowing(InvalidException.class));

Links:

On a side-note: This library also has support for pre- and post-conditions (like assertions really), and it's possible to register your own customized global policy, if needed:

import static org.valid4j.Assertive.*;

require(x, greaterThan(0)); // throws RequireViolation extends AssertionError
...
ensure(r, notNullValue()); // throws EnsureViolation extends AssertionError
keyoxy
  • 4,423
  • 2
  • 21
  • 18
1

Take a look at https://github.com/cowwoc/requirements.java/ (I'm the author). You can override the default exception type using withException() as follows:

new Verifiers().withException(IllegalArgumentException.class).requireThat(name, value).isNotNull();
Gili
  • 86,244
  • 97
  • 390
  • 689
0

Not that I'm aware of. I'd just roll your own to get the behavior you want with a concise invocation, mimicking Guava's implementation but tweaking the exception type.

class Preconditionz {
    public static <T> T checkNotNull(T reference, Object errorMessage) {
        if (reference == null) {
            throw new IllegalArgumentException(String.valueOf(errorMessage));
        }
        return reference;
    }
}

I like to go ahead and import static these really frequently used methods, too, so you can call them super concisely.

import static com.whatever.util.Preconditionz.checkNotNull;

// ...

public void call(Person person) {
    checkNotNull(person, "person");
    checkNotNull(person.getPhone(), "person.phone");
    // ...
}

Depending on your environment, you might want to name it checkNotNull2 so it's easier to add the import via autocompletion in your IDE, or let you use it alongside the standard checkNotNull.

Andrew Janke
  • 23,508
  • 5
  • 56
  • 85
0

I guess I learned something again here on SO thanks to great comments by Olivier Grégoire, Louis Wasserman, CollinD and Captain Man. The standards are ussually a strong and sufficient reason as they make the common language programmers will always understand correctly, but in this particular case I had this little doubt, that maybe this rule set around NPE isn't too ok. Java is an old language and some of its features came up to be a bit unlucky (I don't want to say wrong, that's maybe too strong judgment) - like checked exceptions, although you may also disagree. Now I think that this doubt is resolved and I should:

  • Throw an IllegalArgumentException when in the particular context I can tell why the null value is wrong rather from the business perspective. For instance in the service method public void call(Person person) I know what does it mean to the system that the phone number is null.
  • Throw a NullPointerException when I just know that the null value here is wrong and will sooner or later cause a NullPointerException, but in the particular context I'm unaware what does it mean from the business perspective. The example would be Guavas immutable collections. When you build such and try to add an element of a null value it throws you a NPE. It doesn't understand what this value mean for you, it's too generic, but it just knows it's wrong here, so it decides too tell you this immediately, with some more appropriate message, so that you can recognize the problem more effectively.

Having above in mind I would say the best option to make the assertion in the public void call(Person person) example is like Captain Man suggests:

Preconditions.checkArgument(person.getPhone() != null, "msg");

Check argument is a good name for this method - it's clear that I'm checking the business contract compliance against the person argument and it's clear that I'm expecting IllegalArgumentException if it fails. It's a better name than the Validate.isTrue of Apache Commons. Saying Validate.notNull or Preconditions.checkNotNull on the other hand suggest that I'm checking for a null reference and I'm actually expecting the NPE.

So the final answer would be - there is no such nice library and shouldn't be as this would be confusing. (And Spring Assert should be corrected).

Community
  • 1
  • 1
makasprzak
  • 5,082
  • 3
  • 30
  • 49
-1

You can easily do this:

if (person.getPhone() == null) {
    throw new IllegalArgumentException("Can't call a person that hasn't got a phone");
}

It is clear to other programmers what you mean, and does exactly what you want.

Chronio
  • 757
  • 3
  • 8
  • huh, that's even uglier:) My "ugly" example also does what I wan't and it's also clear. My question was about a library that throws IllegalArgumentException instead NPE (like guava and apache-commons do) for a notNull check. – makasprzak May 04 '15 at 20:07
  • Actually, this is the normal way to check parameters in Java and it doesn't depend on any library. There's no reason not to use this, for as far as I can tell. Also, I don't know of any coding style guideline that says this is ugly. – Chronio May 04 '15 at 20:13
  • My style guideline says so;) But, seriously, I also often do like you suggest. My question is just aboust slightly a different thing. The goal would be to have a short, neat check like Validate.notNull(person.getPhone(), "msg"); – makasprzak May 04 '15 at 20:16