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
