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.

Reply via email to