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

[David Schlosnagle]

On Tue, Jan 25, 2011 at 6:33 PM, Brian Goetz <brian.goetz at oracle.com> wrote:
>>> 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.
>

If you're still open to other possible names for the requireNonNull
method, based on some of the evaluation comments on the highest rated
RFE [1] would you prefer assumeNonNull that throws
NullPointerException when the assumption is violated, otherwise
returns the specified object reference? I honestly don't have a strong
opinion between either requireNonNull or assumeNonNull, but I think it
is at least a small step toward a more comprehensive preconditions
API. As I mentioned before, I'd love to see something along the lines
of Guava's Preconditions or Apache commons-lang's Validate APIs as
part of the JDK, but that is probably best left to JDK 8 to better
flesh out.

[1]: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4449383

- Dave