What's the point of Guava checkNotNull

Question

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?

Answer 1

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();     } 

Answer 2

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.