Code review: 7012540 (java.util.Objects.nonNull() incorrectly named)

Brian Goetz brian.goetz at oracle.com
Tue Jan 25 23:33:47 UTC 2011


>> Additional notes: After much discussion on core-libs-dev, the name
>> requireNonNull() seemed the least objectionable.
>
> I think requireNonNull(x) is confusing.

Remember there's two versions of someModifierNonNull being discussed; 
the one currently in Objects is the precondition-enforcing variety, not 
the postcondition-ensuring variety.  Are we talking about the same thing?

> For those familiar with the "requires/ensures/modifies" triad of verbs as
> a compact way of identifying the preconditions, postconditions, and side
> effects of a method or procedure in a comment, a specification, or a more
> formal design-by-contract framework, "requires" is just wrong.
>
> When analyzing the invocation of foo in your example, the non-nullity of
> s and t are preconditions of foo and therefore postconditions of the
> check method.  Naming the check method "requireNonNull" suggests that
> the check method itself has a precondition that its argument be non-null,
> when in fact it's the check method's postcondition which ensures that
> property.
>
> Since postconditions are labeled "ensures" in the "r/e/m" triad, this
> method should be named "ensureNonNull".

Right, there's precedent for "ensureXxx" methods to actually change the 
state of things to ensure the postcondition, such as ensureCapacity() 
methods in the collection implementations.  Given that a part of the 
motivation for this change was to leave room in the namespace for both 
the precondition-enforcing variety (barf on null) and the 
postcondition-ensuring variety, aka the carpet-sweeping variety ("if it 
is null, make it non-null") ensureNonNull sounds a lot more like the the 
carpet-sweeping version than the version being discussed (barf on null).

The r/e/m framework seems to support require for the throwing version 
(as implemented by this patch): for the throwing version, non-nullity is 
a precondition of the check method (if the condition is not met, error), 
whereas for the carpet-sweeping version, it is a postcondition of the 
check method (if the check method can come up with a reasonable default 
value).  (It happens to be a postcondition of both, but the significant 
behavior and use of the throwing version currently in Objects is to 
enforce an error when the precondition is not met.)  Therefore:

   requireNonNull(x) -> throw if x == null
   ensureNonNull(x)  -> convert x to a non-null value if null

seems like the right taxonomy.



More information about the core-libs-dev mailing list