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

Brian Goetz brian.goetz at oracle.com
Tue Oct 31 14:25:09 UTC 2017


In "View Collections"

Most collections contain elements themselves. By contrast, <i>view 
collections</i>
112 * themselves do not contain elements, but instead rely on a backing 
collection to
113 * store the actual elements.


This should be: "most collections manage the storage for the elements 
they contain.  By contrast, ...".  (All collections contain their 
elements -- just ask the `contains()` method!)

Similarly, in:

Correspondingly, any changes made to
126 * the view collection are written through to the backing collection.


you might want to make the connection that views might "handle" the 
operation by prohibiting it; view collections need not support changes 
if they don't want to.

In

<b>Unmodifiable view collections.</b>

You might want to say explicitly "such as 
Collections.unmodifiable{List,Set,...}" when describing unmodifiable 
view collections.  THat's of course what you're getting at, but it could 
be said more explicitly.

Returns an <a href="#unmodifiable">unmodifiable List</a> containing the 
elements
1045 * of the given Collection, in iteration order.

In the _given collection's_ iteration order.

Returns an <a href="#unmodifiable">unmodifiable Map</a> containing the 
entries
1664 * of the given Map. the given Map must not be null, and it must not 
contain any
1665 * null keys or values. If the given Map is subsequently modified, 
the returned
1666 * Map will not reflect such modifications.

Second sentence starts with lower case later.

I might quibble with "subsequently modified", as there is some ambiguity 
(what if the map is concurrently modified, subsequent to making the 
copyOf() call, but prior to the return from copyOf()?) But I won't ;)

In

296 Collector<T, ?, List<T>> toUnmodifiableList() {
297 return new CollectorImpl<>((Supplier<List<T>>) ArrayList::new, 
List::add,
298 (left, right) -> { left.addAll(right); return left; },
299 list -> (List<T>)List.of(list.toArray()),
300 CH_NOID);

why the intermediate array, and not just use copyOf()?  (Is it because 
you added copyOf later and didn't go back and update this?)  Same for 
other collectors.








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



More information about the core-libs-dev mailing list