Review Request -- 5045147 : When TreeMap is empty explicitly check for null keys in put()
Alan Bateman
Alan.Bateman at oracle.com
Fri Mar 11 21:04:10 UTC 2011
Mike Duigou wrote but nobody heard:
> This is a review request for an issue which was previously committed in 2006 but was quickly withdrawn because it was believed to cause a regression in other software. That removal was mistaken and this fix appears to be bona-fide beneficial.
>
> http://cr.openjdk.java.net/~mduigou/5045147/0/webrev/
>
> Note that this fix impacts both TreeMap and TreeSet. Prior to this fix both have allowed "null" to be added to the collection when the map/set is empty. I've personally run across this issue in usage. Diagnosing and fixing the broken application wasn't initially obvious because of this bug in TreeMap/TreeSet. Only after some frustrating sleuthing were we able to conclude that the problem was in TreeMap.
>
It would be great to get this one fixed, thanks for going through the
history.
The change looks good to me. I guess there isn't really any need to
explicitly check if the comparator is null as it will NPE anyway, and
probably there isn't a need to explicitly check key either as
compare(key,key) will give us the NPE. If you are keeping the explicit
null check then should the braces go?
-Alan.
More information about the core-libs-dev
mailing list