On Tue, 19 Nov 2024 19:25:30 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> Hi Jai, >> >> I am not convinced we need to call this out specifically at the class level >> given the classes implements AutoClosable and close indicates it calls end. >> We will also want to add a Release Note which can help further clarify this >> >> Now that being said, what I would suggest instead is a simple code example >> in the class header which using try-with-resources would be more >> effective/helpful. > > Deflater and Inflater are unusual in that their resources are released by > calling `end()` instead of `close()`. So I think some mention of the > situation is in order. > > The first responsibility is for normative text (before the example) to impose > a requirement on applications that they call `end()` when they are finished. > Something like: > >> To release resources used by this Deflater, applications must call the >> `end()` method. > > This is probably a good place to add a statement about, after `end()` has > been called, all subsequent operations will throw IllegalStateException. > > I don't think we need to include the text about subclasses. I tried drafting > something but it seemed clumsy and unnecessary. But I'm open to other > opinions about whether such text should be included. > > At this point I think it's ok to introduce an `@apiNote` that mentions that a > `close()` method simply calls `end()`, and that this class implements > `AutoCloseable` in order to facilitate use with try-with-resources. And > finally include the big example snippet, which is modified to use > try-with-resources. > > In addition, wording below about finalization and Cleaner should be removed. > The requirement to call `end()` is sufficient, and I don't think there's > anything special about this class that merits a discussion about GC-driven > cleanup. Hello Stuart, I've now updated the PR to follow your recommendations for the class level javadoc of the `Deflater`. Does this look OK? Like you note, I'll update the `Inflater` similarly once we settle on the text for the `Deflater`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19675#discussion_r1853273955
