RFR : 7129185 : (M) Add Collections.{checked|empty|unmodifiable}Navigable{Map|Set}

[Mike Duigou]

Hi Martin;

Thanks, as always, for the feedback!

Louis Wasserman's question about existing methods delegating to newer methods and discussions regarding the serialization impacts with Stuart Marks lead me to reconsider the approach used in revision "3".

For revision "4" I've replaced most of EmptyNavigableMap/Set with instances of Collections.unmodifiableMap/Set(new TreeMap/Set). The number of classes is smaller and serialization considerations for both now and future backwards compatibility are simpler. Also, since the implementation now largely depends upon TreeMap/TreeSet for getting the subMap/Set bounds issues right it's a lot more likely to be error free.

http://cr.openjdk.java.net/~mduigou/JDK-7129185/4/webrev/

On Jun 12 2013, at 11:49 , Martin Buchholz wrote:
> Thanks for doing this.  Arguably, I or Josh or Doug should have done this for jdk6.  It's tedious to get all the details right.

I am very grateful that some work was left undone for me to do. ;-)

> Mostly looks good.
> 
> I suspect the set returned by emptySortedSet is not serialization-compatible between jdk8 and previous jdks.
emptySortedSet() was added in an earlier Java 8 changeset (JDK-4533691, http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2f2f56ac8b82). There is thankfully no backwards compatibility to previous JDK versions though early adopters of JDK 8 will unfortunately encounter serialization compatibility issues as a result of the changes in 7129185

Wanting to get this all finished in the Java 8 iteration is a key motivator.
> Extending EmptySortedSet to also implement NavigableSet ought to be both more compatible and more efficient.  And more tedious!
This shouldn't be necessary since EmptySortedSet appeared in Java 8

> The code fragment below doesn't actually work, cuz WeakHashMap is not a NavigableMap.
Good point. I tried redoing this using a Collections.checkedNavigableMap(new TreeMap()) but quickly decided instead that...
> It's traditional to only @link to a particular destination once per javadoc block.
> +     * any {@link NavigableMap} implementation.  There is no need to use this
> +     * method on a {@link NavigableMap} implementation that already has a
Since both of the NavigableMap implementations in JDK provide NavigableSet implementations this method is not needed. I had created this class as an intermediate step in earlier refactoring but now conclude that it's not needed. I am removing it. If anyone can think of a good reason to include it, please speak now.


Mike

> 
> On Mon, Jun 10, 2013 at 4:36 PM, Mike Duigou <mike.duigou at oracle.com> wrote:
> I've done some further updates based upon feedback. I believe this is now "done" and ready for final review.
> 
> http://cr.openjdk.java.net/~mduigou/JDK-7129185/3/webrev/
> 
> I did find one inconsistency in the implementations SortedSet.headSet and SortedSet.tailSet methods.
> 
> Mike
> 
> 
> On Jun 7 2013, at 10:58 , Mike Duigou wrote:
> 
> > Hello all;
> >
> > I've incorporated feedback from previous rounds and expect to finalize this addition soon.
> >
> > http://cr.openjdk.java.net/~mduigou/JDK-7129185/2/webrev/
> >
> > Any review feedback or suggestions of additional tests welcome.
> >
> > Thanks,
> >
> > Mike
> >
> >
> 
>