We've begun using FindBugs 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.