RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v4]

Hendrik Schreiber hschreiber at openjdk.java.net
Wed Nov 10 11:59:18 UTC 2021


On Wed, 10 Nov 2021 11:40:28 GMT, Anthony Vanelverdinghe <duke at openjdk.java.net> wrote:

>> This is getting too complicated...
>> 
>> It's a code *example* with a very clear comment that explains a best practice:
>> 
>> // A cleaner, preferably one shared within a library
>> private static final Cleaner cleaner = Cleaner.create();
>> 
>> 
>> We cannot really show the best practice in this example without making the example itself more complicated. IMHO, introducing an extra factory method here adds nothing but complexity and makes the example more difficult to understand (that aside, it should probably be something like `MyLibrary.getCleaner()` and not a `createXyz()` method).
>> 
>> I still much more prefer `cleaner = Cleaner.create();` over `cleaner = <cleaner>` (which really is no better in any way, shape or form and creates more questions than it provides answers) or `cleaner = ...`, which again does not answer the question of how to get a cleaner instance—something I asked myself when trying to use the API. In fact *how to get a cleaner instance* is not explained by the current javadocs *at all* and it's something one *must* know to use this API.
>> 
>> Here's our chance to show how it *can* be done, even if it's not ideal, because it does not demonstrate the one-cleaner per library recommendation (only mentions it).
>> 
>> And yes, those ellipsis in the `State` class code—I'd love for them to be gone, too. It would make the example less abstract and easier to understand (what's this state anyway? in most cases it's probably a reference to a state, e.g. a native pointer, i.e. a `long`). But, admittedly, that's difficult.
>> 
>> So, to summarize, there is always a tradeoff between making an example easy to understand, not too complex, but still conveying the most important information and best practices. In the end it's a matter of opinion. In this case, I will stick to my original code change suggestion, because it adds value (answers the question of how to get/create a cleaner instance).
>
> In short: the current code
> 
> 
> // A cleaner, preferably one shared within a library
> private static final Cleaner cleaner = <cleaner>;
> 
> 
> is unhelpful to you, and the proposed code
> 
> 
> // A cleaner, preferably one shared within a library
> private static final Cleaner cleaner = Cleaner.create();
> 
> 
> is confusing to me.
> 
> Trying to find a compromise, I'm merely asking that the comment be clarified:
> 
> 
> // A cleaner (preferably one shared within a library, but for the sake of example, a new one is created here)
> private static final Cleaner cleaner = Cleaner.create();
> 
> 
> What do you think?

Sounds great!
I'll change the code.
Thanks for suggesting this compromise.

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

PR: https://git.openjdk.java.net/jdk/pull/6076


More information about the core-libs-dev mailing list