On Sun, 20 Nov 2022 09:47:35 GMT, Jens Lidestrom <d...@openjdk.org> wrote:

>> 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:
> 
> In the original code, 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...

It depends on *how far* we want to align the behavior. I do see a benefit in 
accepting `s2` being `null`. I do not see a benefit in throwing NPE at a later 
time. Why should an application want to expect that?

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

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

Reply via email to