Code review: 7012540 (java.util.Objects.nonNull() incorrectly named)
Joshua Bloch
jjb at google.com
Wed Jan 26 23:44:05 UTC 2011
I like the name *nonNull*. All other things being equal, shorter is better.
I've used the name *nonNull* for a few years, and it's feels right. To my
mind, *requireNonNull* does a worse job of suggesting that the method
returns its argument.
Josh
On Wed, Jan 26, 2011 at 8:40 AM, Brian Goetz <brian.goetz at oracle.com> wrote:
> The only reason we're even having this discussion now -- as we're well past
> freeze for 7 -- is to prevent the current situation from getting carved into
> stone, where we have a nonNull() precondition-enforcing method in Objects.
> While the correct name for the postcondition-producing version is
> tangentially relevant to that, the short-term goal -- which we keep drifting
> from -- is renaming the precondition-enforcing version so as to *also allow
> room for* a postcondition-producing version later.
>
> Anything more than this is going to get rejected on a "sorry, it's too
> late" basis.
>
> (Its amusing that the goal here was to eliminate a name that was confusing
> because it could apply equally well to two equally valid use cases, and this
> is in fact so confusing that even we cannot be consistent about which
> version we're discussing....)
>
>
> On 1/26/2011 11:01 AM, David Schlosnagle wrote:
>
>> 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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20110126/4ca9fb85/attachment.html>
More information about the core-libs-dev
mailing list