On 29/07/16 12:54, Daniel Fuchs wrote:
Hi,
Here is the new webrev with Chris & James feedback
taken in.
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
This looks good to me Daniel.
-Chris.
best regards,
-- daniel
On 28/07/16 22:55, Daniel Fuchs wrote:
On 28/07/16 21:31, James Perkins wrote:
I just happened to take a glance at this and noticed the remove method
on the KnownLevels doesn't quite look right.
Using Optional.of(nameToLevels.get(mirroredLevel.name)).ifPresent((x) ->
x.remove(this)); will produce an NPE if the level is not present. If the
intention is that the levels may not be present in the list the
Optional.ofNullable() should be used.
Thanks a lot for your keen eyes James!
(sigh!) Why would Optional.of() throw a NPE for null?
But it does...
This wasn't caught by the test because remove() usually
follows a purge() and every call to purge() is synchronized,
so I don't think it could happen, technically - except possibly
for the call purge(KnownLevel). But you're right and the code
is wrong, of course.
ofNullable() it is!
best regards,
-- daniel
