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

[Brent Christian]

FWIW, the HashMap/LinkedHashMap/Hashtable/WeakHashMap changes look good 
to me.

-Brent
On 6/17/13 9:43 PM, Mike Duigou wrote:
Now updated again with additional tests and, as these things go, fixes. I was 
able to use LockStep to exercise the checkedNavigable{Set/Map} and 
synchronizedNavigable{Set/Map} and wrote a basic emptyNavigableMap test. The 
testing looks pretty good now and provides quite complete coverage.

I did have to shut off one small bit of the NavigableMap/LockStep test that 
didn't work for wrapped un-serializable Sets. I don't believe the check it did 
was that important.

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

Mike

On Jun 14 2013, at 16:31 , Mike Duigou wrote:

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-. 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.dui...@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