What's the point of Guava checkNotNull
I'm pretty new to Guava (let's be honest, I'm not "pretty new", I'm a complete rookie on that subject) and so I decided to go through some documentation and got quite amazed while reading this:
com.google.common.base.Preconditions.checkNotNull(...)
I don't get the point of this method. This means that instead of doing :
myObject.getAnything();
(which might cause a NullPointerException
if myObject is null)
I should use
checkNotNull(myObject).getAnything();
which will throw a NullPointerException
if myObject
is null and return myObject
if it is not null.
I'm puzzled and this might be the stupidest question ever but ...
What is the point of this? Those two lines do the exact same thing as for outcomes given any situations I can think of.
I don't even think that the latter is more readable.
So I must be missing something. What is it?
The idea is to fail fast. For instance, consider this silly class:
public class Foo {
private final String s;
public Foo(String s) {
this.s = s;
}
public int getStringLength() {
return s.length();
}
}
Let's say you don't want to allow null values for s
. (or else getStringLength
will throw a NPE). With the class as-is, by the time you catch that null
, it's too late -- it's very hard to find out who put it there. The culprit could well be in a totally different class, and that Foo
instance could have been constructed a long time ago. Now you have to comb over your code base to find out who could possibly have put a null
value there.
Instead, imagine this constructor:
public Foo(String s) {
this.s = checkNotNull(s);
}
Now, if someone puts a null
in there, you'll find out right away -- and you'll have the stack trace pointing you exactly to the call that went wrong.
Another time this can be useful is if you want to check the arguments before you take actions that can modify state. For instance, consider this class that computes the average of all string lengths it gets:
public class StringLengthAverager {
private int stringsSeen;
private int totalLengthSeen;
public void accept(String s) {
stringsSeen++;
totalLengthSeen += s.length();
}
public double getAverageLength() {
return ((double)totalLengthSeen) / stringsSeen;
}
}
Calling accept(null)
will cause an NPE to get thrown -- but not before stringsSeen
has been incremented. This may not be what you want; as a user of the class, I may expect that if it doesn't accept nulls, then its state should be unchanged if you pass a null (in other words: the call should fail, but it shouldn't invalidate the object). Obviously, in this example you could also fix it by getting s.length()
before incrementing stringsSeen
, but you can see how for a longer and more involved method, it might be useful to first check that all of your arguments are valid, and only then modify state:
public void accept(String s) {
checkNotNull(s); // that is, s != null is a precondition of the method
stringsSeen++;
totalLengthSeen += s.length();
}
myObject.getAnything();
(which might cause a NullPointerException if myObject is null)
No... it will throw NPE whenever myObject == null
. In Java, there's no chance of calling a method with null
receiver (a theoretical exception are static methods, but they can and should be always called without any object).
I should use
checkNotNull(myObject).getAnything();
No you should not. This would be rather redundant (Update).
You should use checkNotNull
in order to fail fast. Without it, you may pass an illegal null
to another method, which passes it further, and so on and so on, where it finally fails. Then you can need some good luck to find out that actually the very first method should have refused null
.
The answer by yshavit mentions an important point: Passing an illegal value is bad, but storing it and passing it later is even worse.
Update
Actually,
checkNotNull(myObject).getAnything()
makes sense, too, as you clearly express your intent to not accept any nulls. Without it, someone could think that you forgot the check and convert it into something like
myObject != null ? myObject.getAnything() : somethingElse
OTOH, I don't think the check is worth the verbosity. In a better language, the type system would consider nullability and give us some semantic sugar like
myObject!!.getAnything() // checkNotNull
myObject?.getAnything() // safe call else null
myObject?.getAnything() ?: somethingElse // safe call else somethingElse
for nullable myObject
, while the standard dot syntax would be allowed only when myObject
is known to be non-null.
I have read this whole thread few minutes ago. Nevertheless I was confused why should we use checkNotNull
. Then look over Precondition class doc of Guava and I got what I expected. Excessive use of checkNotNull
will degrade performance definitely.
My thought is checkNotNull
method is worth required for data validation which comes from user directly or very end API to user interaction. It shouldn't be used in each and every methods of internal API because using it you can't stop exception rather correct your internal API to avoid Exception.
According to DOC: Link
Using checkNotNull:
public static double sqrt(double value) {
Preconditions.checkArgument(value >= 0.0, "negative value: %s", value);
// calculate the square root
}
Warning about performance
The goal of this class is to improve readability of code, but in some circumstances this may come at a significant performance cost. Remember that parameter values for message construction must all be computed eagerly, and autoboxing and varargs array creation may happen as well, even when the precondition check then succeeds (as it should almost always do in production). In some circumstances these wasted CPU cycles and allocations can add up to a real problem. Performance-sensitive precondition checks can always be converted to the customary form:
if (value < 0.0) {
throw new IllegalArgumentException("negative value: " + value);
}