add Precondition.checkArgumentNotNull(..) throws IAE

[GoogleCodeExporter]

To check precondions for methods or constructors it would be handy to have a 
equivalent to checkNotNull(..) that throws an IllegalArgumentException.

A good explanation why to throw an IAE in this case rather than a NPE can be 
found here:

http://stackoverflow.com/questions/3881/illegalargumentexception-or-nullpointere
xception-for-a-null-parameter

Original issue reported on code.google.com by chriss....@gmail.com on 30 Jan 2014 at 9:02

[GoogleCodeExporter]

On the same page, this can also be found:
Effective Java 2nd Edition, Item 60: "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-of-range value 
in a parameter representing an index into a sequence, IndexOutOfBoundsException 
should be thrown rather than IllegalArgumentException."

Original comment by SeanPFl...@googlemail.com on 30 Jan 2014 at 10:28

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I think it's an anti-pattern to throw NPE's your self, only the VM should do 
that if you try to access a null reference. Sometimes i get confused when i 
call 3rd party code that throws NPE's deep down in the stack, who knows if its 
a bug or an illegal argument was passed. In this case an IAE left no doubts 
what is going wrong.

Original comment by chriss....@gmail.com on 30 Jan 2014 at 12:06

[GoogleCodeExporter]

Effective Java states this clearly (again in Item 60):

Exception                        | Occasion for Use
IllegalArgumentException         | Non-null parameter value is inappropriate
IllegalStateException            | Object state is inappropriate for method 
invocation
NullPointerException             | Parameter value is null where prohibited
IndexOutOfBoundsException        | Index parameter value is out of range
ConcurrentModificationException  | Concurrent modification of an object has 
been detected where it is prohibited
UnsupportedOperationException    | Object does not support method

Original comment by SeanPFl...@googlemail.com on 30 Jan 2014 at 12:48

[GoogleCodeExporter]

NPE is intended for manual use, not just VM use: "Applications should throw 
instances of this class to indicate other illegal uses of the null object."
http://docs.oracle.com/javase/7/docs/api/java/lang/NullPointerException.html

Additionally, saying that NPE is "an illegal argument" and not "a bug" 
sidesteps the fact that illegal arguments *are* bugs.

"""
This is indeed controversial:

...

A few small advantages to NPE:

1) Checking happens automatically in many cases, so there's less code to write.

2) A small number of APIs (notably AbstractMap.equals) requires that certain 
operations thrown NPE and not IAE. This is a more important rule to us than to 
the average developer because of the number of collections we write.

3) If NullPointerTester accepted IAE, it would mask bugs when an IAE is thrown 
for some other reason (since NPT doesn't necessarily know what are valid values 
for the other parameters):

Foo(String s, Iterable iterator) {
  checkArgument(s.length() > 0);
  // fail to check that iterator isn't null
}
"""

https://groups.google.com/d/msg/guava-discuss/X2dJhNhpIP4/6NEKp39mZlwJ

Original comment by cpov...@google.com on 30 Jan 2014 at 2:41

• Changed state: WorkingAsIntended

[GoogleCodeExporter]

Don't get me wrong, this issue was intended as feature request, not as bug! 

It should be up to the people which way they want to go NPE or IAE. I think 
there are just as many people who want NPE's as IAE's, so it would be good if 
guava could provide an extra method that throws IAE's to satisfy both camps.

Original comment by chriss....@gmail.com on 30 Jan 2014 at 3:29

[GoogleCodeExporter]

Understood. For something that's about personal preference and that's easy to 
implement, we found it easier to pick just one to provide and thus avoid the 
centithreads (like the stackoverflow link you provided) over which users should 
prefer to use :)

Original comment by cpov...@google.com on 30 Jan 2014 at 3:33

[GoogleCodeExporter]

Added to https://code.google.com/p/guava-libraries/wiki/IdeaGraveyard

Original comment by kevinb@google.com on 30 Jan 2014 at 4:37

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07

• Added labels: Restrict-AddIssueComment-Commit