RFR: JDK-8297299 SequenceInputStream should not use Vector [v2]

Markus KARG duke at openjdk.org
Sun Nov 20 09:07:22 UTC 2022


On Sun, 20 Nov 2022 02:16:02 GMT, Jaikiran Pai <jpai at 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


More information about the core-libs-dev mailing list