RFR: 8325304: Several classes in java.util.jar and java.util.zip don't specify the behaviour for null arguments [v5]

[Alan Bateman]

On Wed, 7 Feb 2024 01:52:06 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this doc-only change which updates the javadoc of several classes in `java.util.jar` and `java.util.zip` to specify their behaviour when `null` arguments are passed to the constructor or methods of those classes?
>> 
>> For these updated classes, I have individually checked that they indeed throw a `NullPointerException` when `null` is passed to their constructor or methods. The couple of places where `null` is accepted have been updated to mention that `null` is allowed.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   make <p> usage consistent with other similar usages in the file

This mostly looks okay, just a few comments.

src/java.base/share/classes/java/util/zip/CheckedOutputStream.java line 48:

> 46:      * value to either {@code out} or {@code cksum} will cause
> 47:      * a {@link NullPointerException} to be thrown from the
> 48:      * {@code write} methods of this {@code CheckedOutputStream}.

What is the reason for specifying the NPE in the method description rather than a throws?

src/java.base/share/classes/java/util/zip/DeflaterInputStream.java line 80:

> 78:         // "in" being null isn't allowed. we use a null check for "in"
> 79:         // merely to avoid an unnecessary instance creation of the Deflater
> 80:         // for such erroneous cases.

I see this same comment has been added in 3 places but it's not easy to read and I don't think is needed.

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

PR Review: https://git.openjdk.org/jdk/pull/17728#pullrequestreview-1867349428
PR Review Comment: https://git.openjdk.org/jdk/pull/17728#discussion_r1481197701
PR Review Comment: https://git.openjdk.org/jdk/pull/17728#discussion_r1481189478