Null Creep
I believe it is really important to clearly specify how your classes react to null values. Suppose some class is buried deep in your application code:
public class Company {
private final String id;
private final String name;
public Company(String id, String name) {
this.id = id;
this.name = name;
}
public String getId() {
return id;
}
public boolean equals(Object o) {
// ...hmm...can id be null? Better write some defensive code:
if (id == null) { ... }
...etc
}
}
Perhaps a class like this represents some fundamental concept and is used throughout an entire family of applications. While reviewing the code, you see the equals(...) method checks for a null id. And you wonder…can the id ever be null?
There Must be a Reason
Surely the original programmer put that null check in for a reason. Or maybe not — maybe he generated the equals() and hashCode() methods using an IDE? There are no comments, and you discover everyone using the Company class also checks for null:
public void connectTo(Company c) {
String id = c.getId();
if (id != null) {
...
}
Maybe the programmer who wrote that method just wasn’t sure — since Company has no comments. So in an effort to “be safe”, he checks for null before using the company id.
Creeping Dread…
And what do you do if the id really is null? Oh, crap. Now you have to throw an exception, and someone has to eventually catch it. Everything from the data access layer on up to the GUI has to anticipate the possible error.
Because the first programmer failed to document how Company handles null, EVERYBODY who uses the Company class now has to assume the id might be null. The fact that the original programmer included some null checking in the Company class seems to imply that it might indeed be null.
A Better Solution
If null really is allowed, that’s fine — you just need to document that fact. But if null really is illegal, then fail fast:
public class Company {
private final String id;
private final String name;
public Company(String id, String name) {
if (id == null) throw new IllegalArgumentException("null id");
if (name == null) throw new IllegalArgumentException("null name");
this.id = id;
this.name = name;
}
...etc
That tiny exception now means everybody using the Company class can eliminate their null checking. Which means instead of potentially checking for null and reacting by throwing NullPointerException from dozens of locations scattered about your app(s), the validation occurs in a single location.
For more fun, check out these @Nullable and @NotNull annotations.
Null Creep, sounds like a new Dilbert character.
Findbugs can help chase down Null Creep as well. I seem to remember Bill Pugh and the JetBrains folks having something to do with JSR-305
Great tip, my sentiment exactly. This small habit has an enormous upstream effect on code cleanliness and stability.
The same effect, but in an even nicer way can be achieved with the Preconditions class in the Google Collections.
public Company(String id, String name) {
Preconditions.checkNotNull(id);
Preconditions.checkNotNull(name);
this.id = id;
this.name = name;
}
Steve, you’re right about Google collections. I use it with a static import, which makes the null checks almost vanish into the code:
public Company(String id, String name) {
this.id = checkNotNull(id, “id”);
this.name = checkNotNull(name, “name”);
}
I use a little static helper method that keeps the code clean
static public void notNull(Object… objects) {
for (Object o : objects) {
if (o == null)
throw new NullPointerException();
}
}
Just throw that off in a convenience class somewhere and reference it with static imports. Saves the dependency on Google Collections (although, admittedly I use Google Collections, but I’ve been using this since long before that library was released). The Google Collections version is arguably better for debugging (parameter names), but mine is less of a distraction at write time.
I’ve always loved the idea of @Nullable annotations… anyone know if NetBeans supports something similar? (I find NetBeans to be a much cleaner IDE, but always a year or two behind everyone else with auto hints and things like this)
http://javablog.co.uk/2007/05/07/null-parameters-and-returning-null/
Hell yes, throwing exceptions is what everyone should be doing - let the caller know they’re doing the wrong thing as soon as possible!
So many programmers just check for null and don’t do anything though.
I long ago wrote a NullArgumentException class (which extends IllegalArgumentException) with a static void throwIfNull(Object arg, String parameterName) method
throwIfNull(id, “id”);
throwIfNull(name, “name”);
I use a static import to get access to it.
I am mostly an SQL programmer but you get exactly the same thing in the database world: people allowing NULLs in database columns for no good reason.
Actually, when the code tests for “null”, then what the code does if so should be documented somewhere in the specs…
This NPE solution is nice and simple if your testing (unit or otherwise) can guarantee that all conditions resulting in this NPE have been tested, found and fixed before you ship.
Desktop apps have to be a lot more defensive, however. If this goes into shipping code and occurs in the field, it probably won’t get caught until some high level last defense Throwable catch block, usually at the topmost layer near the event loop. At this point, what can the app to do except quit? Yucch.
I prefer dealing with this at compile time, using the JetBrains @Nullable annotations. It forces you to deal with nulls locally at the level they occur, which is the only place you can recover from them. At that point you can then throw some semantically richer checked exception, giving the next layer up a fighting chance of figuring out what happened.
How about using annotations as described here?
http://www.mcs.vuw.ac.nz/~djp/JACK/