RFR: 8021591 : (s) Additional explicit null checks
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.
More information about the core-libs-dev