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

[David Holmes]

On 23/09/2017 6:37 AM, Stuart Marks wrote:
> On 9/22/17 5:36 AM, Roger Riggs wrote:
>> BTW, I don't think many people are confused by 'immutable' collections 
>> appearing
>> to be mutated because one of the objects in the collection is mutable.
> 
> I'm sure *you* aren't confused by the issues. But I spend a lot of 
> effort explaining these issues to the general public, so I think the 
> change is worthwhile.

I agree with Roger. This seems to conflate immutability of the 
collection with immutability of the elements in the collection. I don't 
see a definition of mutability/immutability that would imply that an 
immutable collection must have immutable elements. To me the notion of 
mutability is for the collection itself - the group of objects contained 
- not the objects themselves. Have I missed this definition of 
"immutable" somewhere? Otherwise it might be clearer to explain that an 
"immutable collection" may have different meanings to different people.

I also find the use of "As above" in this sentence:

"As above, an unmodifiable view collection is not necessarily immutable."

inappropriate, because "above" you are referring to the mutability of 
the elements of the collection and in the current context you are 
referring to the mutability of the collection itself. And this is quite 
a different issue to the one "above".

Cheers,
David

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