On Sun, 20 Nov 2022 09:14:32 GMT, Markus KARG <d...@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 82:
> 
>> 80:      * @param   s2   the second input stream to read.
>> 81:      */
>> 82:     public SequenceInputStream(InputStream s1, InputStream s2) {
> 
> BTW, what is your opinion @jaikiran  and @AlanBateman: We could simplify the 
> 2-arg constructor by calling `this(...)` instead of repeating the 1-arg 
> constructor's implementation here. Is that a *preferred* or a *disliked* 
> pattern in OpenJDK?

The updated code now changes the behaviour in the other direction:

Previously, if `s2` was null a NPE was thrown in `peekNextStream` when `s1` was 
exhausted.

In the current code, `s2` is silently ignored if it is null.

A safer alternative that preserves the behaviour of nulls seems to be the 
replace `List.of` with `Arrays.asList`.

These subtle changes in behaviour demonstrates the problem with even trivial 
updates to legacy code...

-------------

PR: https://git.openjdk.org/jdk/pull/11249

Reply via email to