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

[Stuart Marks]

On 10/31/17 11:24 AM, Roger Riggs wrote:
> Hi Stuart,
> 
> Collection.java:
> 
> The mix of "should" and "must" in Collection.java can be confusing.
> Typically "should" is interpreted as "must" from a conformance point of view.

"Must" is a requirement, and "should" is a recommendation to implementors, and 
advice to clients that some property often but doesn't necessarily hold.

> For example,
> 
> 147 * <p>An <i>unmodifiable collection</i> is a collection, all of whose
> 148 * mutator methods (as defined above) are specified to throw
> 149 * {@code UnsupportedOperationException}. Such a collection thus cannot be
> 150 * modified by calling any methods on it. For a collection to be properly
> 151 * unmodifiable, any view collections derived from it *must *also be 
> unmodifiable.
> 152 * For example, if a List is unmodifiable, the List returned by
> 153 * {@link List#subList List.subList} *should *also be unmodifiable.
> 
> That leaves room for a non-compliant collection which could not technically be 
> called "unmodifiable".

I agree that "should" is poor here; I'll reword this to say that the returned 
list "is also unmodifiable."

> 138 * does not support the operation. Such methods *should *(but are not required
> 139 * to) throw an {@code UnsupportedOperationException} if the invocation would
> 140 * have no effect on the collection. For example, invoking the
> 141 * {@link #addAll addAll} method on a collection that does not support
> 142 * the {@link #add add} operation *should *throw the exception if
> 143 * the collection passed as an argument is empty.
> 
> These two sentences both allow not throwing UOE and requiring UOE. Can they be 
> worded to be consistent (or similarly qualified.)

For this case, implementations are recommended to throw UOE, but they are not 
required to do so. Some implementations will throw, while others will not throw. 
There are examples of both in the JDK. I'll clarify the example, but I really do 
mean "should" here.

> Set.java: 706:  Would using "unique elements" clarify the contents of the result?
> Saying "Duplicate elements are permitted," may be misleading.
> Perhaps "The given Collection may contain duplicate elements"...

I'll clarify this to say that if the given Collection contains duplicate 
elements, an arbitrary element of the duplicates is preserved.

> Similarly in Collectors.to toUnmodifiableSet:
> "Duplicate elements are allowed," can be misleading, especially the word allowed.

Will update similarly.

> Collectors.toUnmodifiableMap
>       * If the mapped keys
> 1474      * might have duplicates, use {@link #toUnmodifiableMap(Function, 
> Function, BinaryOperator)}
> 1475      * instead.
> 
> It would helpful to say why to use the version with the BinaryOperator, for 
> example to say"
> "To determine which of the duplicate elements to retain".  or similar.

Will add words about handling the merging of values.

> Collectors:1606:  impl note:  With three different NPE throws, it is helpful to 
> use the 2-arg
> form of requireNonNull to indicate which parameter offends.

Will update.

Thanks,

s'marks

> 
> Roger
> 
> On 10/30/2017 6:50 PM, Stuart Marks wrote:
>> (also includes 8184690: add Collectors for collecting into unmodifiable List, 
>> Set, and Map)
>>
>> Hi all,
>>
>> Here's an updated webrev for this changeset; the previous review thread is here:
>>
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-September/049261.html
>>
>> This webrev includes the following:
>>
>> * specification revisions to provide clearer definitions of "view" 
>> collections, "unmodifiable" collections, and "unmodifiable views"
>>
>> * new List.copyOf(), Set.copyOf(), and Map.copyOf() "copy factory" methods
>>
>> * new Collectors.toUnmodifiableList, Set, and Map methods
>>
>> * tests for the new API methods
>>
>> I've added some assertions that require some independence between the source 
>> collection (or map) and the result of the copyOf() method.
>>
>> I've made a small but significant change to Set.copyOf compared to the 
>> previous round. Previously, it specified that the first of any equal elements 
>> was preserved. Now, it is explicitly unspecified which of any equals elements 
>> is preserved. This is consistent with Set.addAll, Collectors.toSet, and the 
>> newly added Collectors.toUnmodifiableSet, none of which specify which of 
>> duplicate elements is preserved.
>>
>> (The outlier here is Stream.distinct, which specifies that the first element 
>> of any duplicates is preserved, if the stream is ordered.)
>>
>> I've also made some minor wording/editorial changes in response to suggestions 
>> from David Holmes and Roger Riggs. I've kept the wording changes that give 
>> emphasis to "unmodifiable" over "immutable." The term "immutable" is 
>> inextricably intertwined with "persistent" when it comes to data structures, 
>> and I believe we'll be explaining this forever if Java's "immutable" means 
>> something different from everybody else's.
>>
>> Webrev:
>>
>>     http://cr.openjdk.java.net/~smarks/reviews/8177290/webrev.1/
>>
>> Bugs:
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8177290
>>         add copy factory methods for unmodifiable List, Set, Map
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8184690
>>         add Collectors for collecting into unmodifiable List, Set, and Map
>>
>> Thanks,
>>
>> s'marks
>