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