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
>> >
>> >
>> 
>> 
> 

Reply via email to