On Sun, 20 Nov 2022 02:16:02 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Markus KARG has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> allowing s2 to be null
>
> 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`.
Thank you for this quick review and bringing up this topic! I will follow-up on
this in the below opened by @AlanBateman.
-------------
PR: https://git.openjdk.org/jdk/pull/11249
