Hi Mike,
Looking mainly at the docs not the operational semantics of null
handling ...
On 17/10/2013 9:52 AM, Mike Duigou wrote:
On Oct 16 2013, at 05:34 , Paul Sandoz <paul.san...@oracle.com> wrote:
On Oct 16, 2013, at 1:52 PM, David Holmes <david.hol...@oracle.com> wrote:
Perhaps HashMap's implementations should throw CME?
Perhaps, seems to be going beyond the call of duty. My inclination is not to
bother. It becomes most relevant with forEach since the consumer will have
side-effects that might make it easier to unintentionally slip in a
modification to the map itself.
I think there is a lot to be said for consistency.
Yes, i was proposing to consistently not support it for non-traversal methods
:-)
I have prepared an updated webrev removing all of the non-traversal CME
throwing.
Hmmmm ... see below ...
http://cr.openjdk.java.net/~mduigou/JDK-8024688/2/webrev/
Map.java:
The implNote for computeIfAbsent should be modified to match the
implementation. Ditto for computeIfPresent. Ditto for compute, merge
etc! Once these implementations have stabilized we need to check what
the implNote says. It makes no sense to me for the impl note to describe
anything other than the core logic of the actual implementation -
particularly referring to putIfAbsent when put is used, or replace when
put is used.
HashMap.java:
1234 if(old.value != null)
Space
ConcurrentMap.java:
247 * @throws ClassCastException {@inheritDoc}
248 * @throws NullPointerException {@inheritDoc}
249 * @throws ClassCastException {@inheritDoc}
CCE is repeated.
274 * contain null values and get() returning null unambiguously
means the key
get() should be in code font as per line #69. Ditto for line 300, 332
and 395.
I now see that those Map implNotes were written with ConcurrentMap in
mind so that it can just tag on the note about retries. But this seems
wrong to me - each should have its own implNotes reflecting the true
implementation.
It does bother me to be throwing out "good information" by not throwing the
CMEs but I'm willing to go with the flow. As a practical matter later reintroduction of
even valid error detection would almost certainly be difficult.
(https://bugs.openjdk.java.net/browse/JDK-5045147 for one example).
... it still concerns me that a function object could mutate the map and
so trigger a CCE.
The patch also fixes up missing @throws and @since from the ConcurrentMap
implementations.
I think you have a one-line conflict with Henry's latest @since update.
Cheers,
David
Mike
