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 <[email protected]> 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
>> >
>> >
>>
>>
>