RFR: 8021591 : (s) Additional explicit null checks

Paul Sandoz paul.sandoz at oracle.com
Mon Jul 29 18:12:59 UTC 2013


On Jul 29, 2013, at 2:46 PM, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>> diff --git a/src/share/classes/java/util/Map.java b/src/share/classes/java/util/Map.java
>> --- a/src/share/classes/java/util/Map.java
>> +++ b/src/share/classes/java/util/Map.java
>> @@ -804,6 +804,10 @@
>>       *     return false;
>>       * }</pre>
>>       *
>> +     * @implNote The default implementation does not throw NullPointerException
>> +     * for maps that do not support null values if oldValue is null unless
>> +     * newValue is also null.
>> 
>> 
>> Is that really more a clarification of the default impl specification?
> 
> I thought implNote was to be used for default implementations, but it appears that it is implSpec?
> 

Correct, @implNote can be used for an implementation of anything not just a default implementation; it signals some non-normative stuff. Whereas @implSpec defines the behaviour for a particular implementation whose actual code could be implemented differently by another JDK implementation, it could equally apply to methods on classes too e.g. those abstract collection classes


>> 
>> 
>> -     * @throws NullPointerException if a specified key or value is null,
>> +     * @throws NullPointerException if a specified key or newValue is null,
>>       *         and this map does not permit null keys or values
>> +     * @throws NullPointerException if oldValue is null and this map does not
>> +     *         permit null values
>> +     *         (<a href="Collection.html#optional-restrictions">optional</a>)
>> 
>> 
>> More curious than anything else, is it fine to have two declarations of NPE here?
> 
> Having two @throws looks a little strange to me. Putting it together???
> 
> * @throws NullPointerException if a specified key or newValue is null,
> *         and this map does not permit null keys or values. If
> *         oldValue is null and this map does not permit null values
> *         (<a href="Collection.html#optional-restrictions">optional</a>).
> 
> Is this even right? Should the implNote not be part of the NPE throws?

It seems fine to state:

     * @throws NullPointerException if the specified key, newValue or oldValue is null,
     *         and this map does not permit null values (...)

And let the @implSpec refine things for the default implementation.


> Do you want replace to be used to put an initial value ( where previously unset (null) )?
> 

Not according to what is currently defined:

     * Replaces the entry for the specified key only if currently
     * mapped to the specified value.

Paul.




More information about the core-libs-dev mailing list