Hi Brent,
The implementation looks good.
Documentation-wise i think it needs some adjustment:
- the @apiNote should be promoted to "normal" documentation as we want to make
a stronger statement.
- I think the @implNote on Map should be merged into the @implSpec. How about:
<p>The default implementation makes no guarantees about detecting if the
mapping function modifies
this map during computation and, if appropriate, reporting an error.
Non-concurrent implementations should override this method and, on a
best-effort basis, throw a
{@code ConcurrentModificationException} if it is detected that the mapping
function modifies this
map during computation.
Concurrent implementations should override this method and, on a best-effort
basis, throw a
{@code IllegalStateException} if it is detected that the mapping function
modifies this map
during computation and as a result computation would never complete.
?
- the @implSpec on HashMap etc. should be promoted to "normal" documentation:
<p>This method will, on a best-effort basis, throw a {@link
ConcurrentModificationException} if it is detected
that the mapping function modifies this map during computation.
?
Testing-wise can you use a TestNG data supplier? then you can separate out the
larger test into the smaller tests. Meaning we test all combinations regardless
of which fail, and each will be reported so there is no need to print out test
info. If you need help with that i can provide an example.
Thanks,
Paul.
On Mar 18, 2015, at 6:46 PM, Brent Christian <brent.christ...@oracle.com> wrote:
> Hi,
>
> Please review my changes for 8071667 : "HashMap.computeIfAbsent() adds entry
> that HashMap.get() does not find"
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8071667
>
> Webrev+specdiff:
> http://cr.openjdk.java.net/~bchristi/8071667/webrev.2/
>
>
> The fix is to detect structural changes made by the mapping functions passed
> to compute() and friends, and to throw a ConcurrentModificationException.
> This prevents possibly adding entries in the wrong hash bin.
>
> I have updated the docs based on the prior discussion [1], making use of the
> new @implSpec/etc tags, as seemed sensible to me. Suggestions welcome.
>
>
> Automated test run is clean. If the new docs look good, I'll start a CCC.
>
> Thanks,
> -Brent
>
> 1.
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031245.html
>
