RFR: 8225763: Inflater and Deflater should implement AutoCloseable [v5]

Jaikiran Pai jpai at openjdk.org
Fri Nov 15 11:23:28 UTC 2024


On Thu, 14 Nov 2024 17:29:06 GMT, Lance Andersen <lancea at openjdk.org> 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


More information about the core-libs-dev mailing list