RFR: 8001647: In-place methods on Collection/List

Akhil Arora akhil.arora at oracle.com
Fri Apr 19 18:37:04 PDT 2013


On 04/18/2013 12:36 PM, Mike Duigou wrote:
> Hi Akhil;
>
> List.sort::
>
> - @since tag is in a strange location.
>
> - The (optional) on IAE is in a strange position and not linked like the others.

fixed both

> AbstractList::
>
> - Should we consider adding overrides for default methods here even if our impls wouldn't use them? We could at least add modCount checking.

I tried, but could not do anything here more than what the default is 
doing, so left it alone

> Tests::
>
> - Lots of enhancement here since my last review. Good Job!
>
> - Wrong GPL license. Tests don't get Classpath exemption.

fixed

> - In Map.Defaults (around line 539 in http://cr.openjdk.java.net/~mduigou/JDK-8010122/6/webrev/test/java/util/Map/Defaults.java.html) I found it useful to generate an implementation of the base interface to directly test the default methods implementations. You may want to add something similar to CollectionExtensionMethodsTest.

the default methods are exercised (and tested) by the classes that do 
not provide optimized defaults, so i think we do have coverage for the 
pure default methods

> - You may want to include a Collections.newSetFromMap in DataProvider.

done

> - I am now preferring the Iterator<Object[]> return from DataProvider though I haven't made an on-demand provider yet. You could also mark your DataProvider as ", parallel = true"

added parallel

> On Apr 18 2013, at 11:49 , Akhil Arora wrote:
>
>> Looks like the stars are aligning on getting on this into TL... the refreshed webrev is -
>>
>> http://cr.openjdk.java.net/~akhil/8001647.8/webrev/
>>
>> Please review
>>
>> Thanks
>>
>> On 12/10/2012 09:31 PM, Akhil Arora wrote:
>>> http://cr.openjdk.java.net/~akhil/8001647.3/webrev/
>>>
>>> - now with synchronized and unmodifiable wrappers in Collections.java
>>> for the default methods being added
>>>
>>> On 12/10/2012 01:48 PM, Akhil Arora wrote:
>>>> Updated with yours and Alan's comments -
>>>>
>>>> http://cr.openjdk.java.net/~akhil/8001647.2/webrev/
>>>>
>>>> - removed null check for removeSet
>>>> - cache this.size in removeAll, replaceAll
>>>>      (for ArrayList, Vector and CopyOnWriteArrayList)
>>>> - calculate removeCount instead of BitCount.cardinality()
>>>> - removed unnecessary @library from test support classes
>>
>



More information about the lambda-dev mailing list