9

Looking at the implementation for Stream#toList, I just noticed how overcomplicated and suboptimal it seemed.

Like mentioned in the javadoc just above, this default implementation is not used by most Stream implementation, however, it could have been otherwise in my opinion.

The sources

/**
 * Accumulates the elements of this stream into a {@code List}. The elements in
 * the list will be in this stream's encounter order, if one exists. The returned List
 * is unmodifiable; calls to any mutator method will always cause
 * {@code UnsupportedOperationException} to be thrown. There are no
 * guarantees on the implementation type or serializability of the returned List.
 *
 * <p>The returned instance may be <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>.
 * Callers should make no assumptions about the identity of the returned instances.
 * Identity-sensitive operations on these instances (reference equality ({@code ==}),
 * identity hash code, and synchronization) are unreliable and should be avoided.
 *
 * <p>This is a <a href="package-summary.html#StreamOps">terminal operation</a>.
 *
 * @apiNote If more control over the returned object is required, use
 * {@link Collectors#toCollection(Supplier)}.
 *
 * @implSpec The implementation in this interface returns a List produced as if by the following:
 * <pre>{@code
 * Collections.unmodifiableList(new ArrayList<>(Arrays.asList(this.toArray())))
 * }</pre>
 *
 * @implNote Most instances of Stream will override this method and provide an implementation
 * that is highly optimized compared to the implementation in this interface.
 *
 * @return a List containing the stream elements
 *
 * @since 16
 */
@SuppressWarnings("unchecked")
default List<T> toList() {
    return (List<T>) Collections.unmodifiableList(new ArrayList<>(Arrays.asList(this.toArray())));
}

My idea of what would be better

return (List<T>) Collections.unmodifiableList(Arrays.asList(this.toArray()));

Or even

return Arrays.asList(this.toArray()));

IntelliJ's proposal

return (List<T>) List.of(this.toArray());

Is there any good reason for the implementation in the JDK sources?

Yassin Hajaj
  • 21,337
  • 9
  • 51
  • 89
  • 4
    This was [discussed here under comments](https://stackoverflow.com/questions/65741773/would-stream-tolist-perform-better-than-collectors-tolist#comment116293110_65741773) as well. – Naman Apr 05 '21 at 03:32
  • 6
    You're misreading the code. This is not the implementation you think it is; this is the default implementation used for third-party implementations of `Stream` that do not provide an implementation of `toList`. The JDK does something very different; see `ReferencePipeline::toList` for the actual implementation. – Brian Goetz Apr 05 '21 at 18:47

1 Answers1

19

The toArray method might be implemented to return an array that is then mutated afterwards, which would effectively make the returned list not immutable. That's why an explicit copy by creating a new ArrayList is done.

It's essentially a defensive copy.

This was also discussed during the review of this API, where Stuart Marks writes:

As written it's true that the default implementation does perform apparently redundant copies, but we can't be assured that toArray() actually returns a freshly created array. Thus, we wrap it using Arrays.asList and then copy it using the ArrayList constructor. This is unfortunate but necessary to avoid situations where someone could hold a reference to the internal array of a List, allowing modification of a List that's supposed to be unmodifiable.

Jorn Vernee
  • 31,735
  • 4
  • 76
  • 93