On Sat, 19 Nov 2022 21:36:56 GMT, Markus KARG <d...@openjdk.org> wrote:
> There is no need to use a temporary Vector within the constructor of
> SynchronizedInputStream, as more efficient (non-synchronized) alternative
> code (like List.of) will do the same in possibly less time. While the
> optimization is not dramatic, it still makes sense to replace Vector unless
> synchronization is really needed.
src/java.base/share/classes/java/io/SequenceInputStream.java line 43:
> 41: * on the last of the contained input streams.
> 42: *
> 43: * @author Arthur van Hoff
Good catch. Looking at some other files in the JDK, your change here looks
correct.
src/java.base/share/classes/java/io/SequenceInputStream.java line 83:
> 81: */
> 82: public SequenceInputStream(InputStream s1, InputStream s2) {
> 83: e = Collections.enumeration(List.of(s1, s2));
Hello Markus,
I think removing the `Vector` usage looks fine. The use of `List.of` in this
constructor introduces a small change in behaviour of this constructor if the
constructor is being passed a `null` `Inputstream`.
Previously, before this change, if the first `InputStream` parameter was null
this constructor would throw a `NullPointerException` (because the call to
`peekNextStream()` in this constructor throws that). However, if the second
`InputStream` was `null` then the constructor would return back fine. Only
later when the `SequenceInputStream` was used and it was time to move to use
the second `InputStream`, it would throw the `NullPointerException` (from the
same `peekNextStream()`).
Now, with the use of `List.of(s1, s2))`, the `NullPointerException` will get
thrown early if either of these two parameters is `null`.
Whether or not this change in behaviour related to `NullPointerException` is
acceptable (backed by a CSR) is something that others more familiar with this
area would be able to tell. But I think it may not be desirable because
depending on how the application might have been using the
`SequenceInputStream` instance they may not have been encountering the
`NullPointerException` (for the second param) if they never read the
`SequenceInputStream` completely.
You could still remove the `Vector` usage from this code though, by
constructing the collection differently instead of `List.of`.
-------------
PR: https://git.openjdk.org/jdk/pull/11249
