RFR : 7129185 : (L) Add Collections.{checked|empty|unmodifiable}Navigable{Map|Set}
Brent Christian
brent.christian at oracle.com
Tue Jun 18 16:57:19 UTC 2013
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.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
>>>>
>>>>
>>>
>>>
>>
>
More information about the core-libs-dev
mailing list