Hello all; I've continued to refine the previously posted patch and think it's now quite close to complete. I am going to scour the tests looking for additional conditions that need to be checked but this seems to be very nearly finished.
http://cr.openjdk.java.net/~mduigou/JDK-7129185/1/webrev/ Since the last review I have added a newNavigableSetFromNavigableMap() implementation which I needed temporarily as I refactored out duplication in BoundedEmptyNavigableMap. I can either include this as part of this issue (it seems related enough), make a separate issue, or remove it. Comments and feedback welcome. Thanks! Mike On Apr 29 2013, at 18:54 , Mike Duigou wrote: > Hello all; > > This is a non-integration code review. I am picking this patch up after > ignoring it for most of the last year. I've recently expanded the regression > tests to, I believe, handle almost all of the new code paths added by this > patch. > > http://cr.openjdk.java.net/~mduigou/JDK-7129185/0/webrev/ > > This issue is a follow-on to JDK-4533691 which added emptySortedSet(). In > addition to adding support for NavigableSet/Map this patch also corrects > differences between the behaviour of: > > Set<Integer> uts = Collections.unmodifiableNavigableSet(new TreeSet()) > > and > > Set<Integer> es = Collections.emptyNavigableSet() > > involving bounded sub-ranges. At this point I believe that "uts" and "es" > will be operationally indistinguishable. emptyNavigableSet() will still be > more efficient though as it is a singleton and doesn't generally(*) consume > additional resources for each instance. The asterisk next to generally comes > from the bounded sub-ranges functionality. Sub ranges of empty SortedSet and > NavigableSet will no longer be the singleton. They are instead instances > which capture the range. > > Because so much time has passed since this issue originally surfaced I'm > concerned that I might be forgetting something. I do know that I still need > to create an EmptyNavigableMap unit test and add serialversionid to all the > new classes but does anything else seem to be missing either in terms of the > implementation or the tests? > > Mike
