RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

[Stuart Marks]

On 9/21/17 12:16 PM, Roger Riggs wrote:
> Hi Stuart,
>
> The new text in Collections reads more like an @apinote than a specification.
> Are there any enforceable  assertions there?

There probably aren't any testable assertions, but the new text does contain 
normative definitions. Usually this stuff appears on package docs. Unfortunately 
java.util is shared with other stuff unrelated to collections, so the overview 
material isn't there. It ended up in java.util.Collection (which is the "lead" 
interface of the Collections Framework) so I've added the new material there.

The @apiNote tag is for non-normative (informative) material, so I don't think 
it's appropriate for this new material.

> I think the markup used to refer to unmodifiable XXX reads better as a link in
> the text (as in Collection#unmodifableCollection)
> than as a second sentence (as in List#of()).
> A consistent treatment in all class would be a plus.

OK, I can adjust these.

> The implementations of the Collectors are very inefficient, first creating a
> mutable version,
> then extracting to an array, and then constructing the final object.  So much
> garbage is created, especially for small n.

Yes, these are preliminary implementations, to which many optimizations can be 
applied.

s'marks

> Thanks, Roger
>
>
> On 9/21/2017 2:55 PM, Stuart Marks wrote:
>>
>>
>> On 9/21/17 5:42 AM, Alan Bateman wrote:
>>> On 21/09/2017 01:02, Stuart Marks wrote:
>>>> http://cr.openjdk.java.net/~smarks/reviews/8177290/webrev.0/
>>> I read through the updated/new definitions and they read well.
>>
>> Great.
>>
>>> For the copyOf methods then I can't immediately tell from the javadoc if the
>>> given collection can contain null elements. Taking List.copyOf as an example
>>> where coll may be null or it may contain null elements. The javadoc does link to
>>> "Unmodifiable lists" where it specifies the characteristics of the lists
>>> returned by the static factory methods - these include disallowing null
>>> elements. So I think this needs to be clarified.
>>
>> Agreed, I'll work on some clarifications here, and also disallow null for the
>> argument itself.
>>
>>> Minimal implementation is okay to get started but what is the reason not to
>>> include some basic tests?
>>
>> Sorry, I should have been more clear about this. The changeset is clearly not
>> ready to go in as it stands. I wanted to get an initial review of the
>> specifications going, then file a CSR request, etc. while continuing to work
>> on tests and better implementations. I'll post a subsequent review when
>> they're ready.
>>
>> Thanks.
>>
>> s'marks
>>
>