Check parameters annotated with @Nonnull for null?
- by David Harkness
We've begun using FindBugs with and annotating our parameters with @Nonnull appropriately, and it works great to point out bugs early in the cycle. So far we have continued checking these arguments for null using Guava's checkNotNull, but I would prefer to check for null only at the edges--places where the value can come in without having been checked for null, e.g., a SOAP request.
// service layer accessible from outside
public Person createPerson(@CheckForNull String name) {
return new Person(Preconditions.checkNotNull(name));
}
...
// internal constructor accessed only by the service layer
public Person(@Nonnull String name) {
this.name = Preconditions.checkNotNull(name); // remove this check?
}
I understand that @Nonnull does not block null values itself.
However, given that FindBugs will point out anywhere a value is transferred from an unmarked field to one marked @Nonnull, can't we depend on it to catch these cases (which it does) without having to check these values for null everywhere they get passed around in the system? Am I naive to want to trust the tool and avoid these verbose checks?
Bottom line: While it seems safe to remove the second null check below, is it bad practice?
This question is perhaps too similar to Should one check for null if he does not expect null, but I'm asking specifically in relation to the @Nonnull annotation.
