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

[Rémi Forax]

On 01/27/2011 09:38 AM, Stephen Colebourne wrote:
> As I said before, removing this method is the best option now. Get it
> right in v8.
> Stephen

I think we can't.
This method is already used at many place in the JDK.

About the name, I propose: 
iUsedToUseGetClassHereButNodobyWasAbleToUnderstand()

Rémi

>
> On 27 January 2011 05:05,<mark.reinhold at oracle.com>  wrote:
>> Executive summary: requireNonNull is the preferred name.
>>
>>> Date: Tue, 25 Jan 2011 18:33:47 -0500
>>> From: brian.goetz at oracle.com
>>> mark.reinhold at oracle.com wrote:
>>>> 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?
>> Yes, i.e., the former.
>>
>>>> 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).
>> I was actually arguing that the barfing version should be named
>> ensureNonNull.
>>
>> (Wow, am I actually writing about barfing methods?)
>>
>>> 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.
>> It depends on your perspective.
>>
>> In a nested method invocation, the postconditions of the inner method
>> must relate to the preconditions of the outer method.  That is, for the
>> expression
>>
>>     foo(bar(x))
>>
>> to be correct then the postconditions of the bar method must imply the
>> preconditions of foo method.
>>
>> Now suppose that bar is intended to be the barf-on-null method.
>>
>> If we consider how to name bar on its own, outside of its intended usage
>> as in this example, then it's natural (for me, anyway) to conclude that
>> "ensuresNonNull" is the right name.  What's important about bar is its
>> postcondition, namely that its return value (x) is not null.  Naming it
>> "requireNonNull" seems backwards on this view, because "requires" is
>> always about preconditions, and it's definitely not a precondition of
>> this method that its argument not be null.
>>
>> If, by contrast, we consider how to name bar in the context of the above
>> example then it's easy to see that "requireNonNull" is the better name,
>> because what's important in the overall expression is satisfying foo's
>> precondition that its argument not be null, and so
>>
>>     foo(requireNonNull(x))
>>
>> reads very fluently.
>>
>> A little utility method like this will never (well, hardly ever) be used
>> in isolation, so I agree now that it makes sense to prefer the second
>> form, i.e., requireNonNull.
>>
>> I'm not sure I agree that the corresponding carpet-sweeping method is
>> best named ensureNonNull; treating that as a conversion, i.e., asNonNull,
>> may be a better choice.  That, however, is a discussion for another
>> release.
>>
>> - Mark
>>