On Thu, 14 Nov 2024 17:29:06 GMT, Lance Andersen <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 10 additional
>> commits since the last revision:
>>
>> - merge latest from master branch
>> - update tests to match the new specification
>> - Stuart's review - update the close() and end() expectations
>> - Stuart's review - improve class level javadoc
>> - merge latest from master branch
>> - merge latest from master branch
>> - Chen's suggestion - improve code comment
>> - convert the tests to junit
>> - fix whitespace
>> - 8225763: Inflater and Deflater should implement AutoCloseable
>
> src/java.base/share/classes/java/util/zip/Deflater.java line 66:
>
>> 64: * usage with try-with-resources, this class implements {@link
>> AutoCloseable}. The
>> 65: * {@link #close()} method of this class calls {@code end()} to clean up
>> its
>> 66: * resources. Subclasses are responsible for the cleanup of resources
>
> As part of the clarification, do we need to state the verbiage above
> regarding try-with-resources specifically?
>
> I did a quick scan of some of the other classes implementing AutoClosable and
> did not see many cases of that in the documentation most likely due to that
> behavior is clear in the documentation for AutoClosable
Stuart in one of his review comments in this PR had noted that:
> I think the class specification needs to be clearer about the positioning of
> end() and close(). The end() method has done the real work of "closing" since
> the classes were introduced. We're retrofitting them to to have a close()
> method that's essentially just a synonym for end(), and it's still perfectly
> legal for clients and subclasses to call end() instead of close(). I think we
> need to say that close() is present mainly to support AutoCloseable and
> try-with-resources.
The current wording of that class level javadoc is my attempt to implement that
suggestion. Do you think we can word it differently and yet convey the purpose
of close()?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19675#discussion_r1843618146
