On Tue, 9 Nov 2021 11:14:37 GMT, Anthony Vanelverdinghe <d...@openjdk.java.net>
wrote:
>> Let me add, why I have raised this issue.
>>
>> I was going to migrate some older code which uses the `finalize()` method to
>> the `Cleaner` mechanism. New it it, there seemed to be two pitfalls:
>>
>> 1. Understanding the whole "don't capture an instance reference in your
>> state object"
>> 2. Copying the example (which was in a non-working state, due to pseudo
>> code) and making it work for me
>>
>> With the improvement suggestions, I was trying help people who *only* read
>> the code sample (many do), to become aware of 1. and help them getting going
>> with 2, simply because it's something they can copy and run.
>
>> When I see `<cleaner>`, I'm just wondering what those <> type operators are
>> good for here...
>
> What about just replacing `<cleaner>` with `...` then? The `State`
> constructor and its invocation also have an ellipsis.
>
>> BUT, at least it's a working example and not some pseudo code.
>
> The example is still not compilable due to the remaining ellipses.
>
>> We do want to move to working example code long term, don't we?
>
> I agree that examples should be compilable *in the raw Javadoc*. However, in
> the rendered Javadoc, using ellipses is a well-understood way to keep
> examples concise and devoid of irrelevant and/or usecase-dependent code.
> Moreover, when developers copy-paste the example, they'll immediately be
> pointed to all the places where they need to fill in the blanks, make a
> choice for a trade-off, etc. On the other hand, by hard-coding a (suboptimal)
> choice, developers who merely copy-paste the example are unlikely to
> reconsider the trade-off.
>
>> The new example Cleaner instance _is_ shared, though on a pretty small scale
>> (just among instances of CleaningExample).
>
> True, but my point was that the comment says "shared *within a library*". So
> to me it's confusing to have a comment saying "it's preferred to do A", and
> then have the code do B on the next line.
>
> So I propose to either:
> * revert the current change & simply replace `<cleaner>` with `...`
> * update the comment to say: "A cleaner (preferably one shared within a
> library, but for the sake of example, a new one is created here)"
>
> Actually, to have the line be compilable, and at the same time (attempt to)
> force developers to consider the trade-off, it could be changed to something
> like:
>
>
> private static final Cleaner cleaner = createCleaner();
>
> private static Cleaner createCleaner() {
> // A cleaner, preferably one shared within a library
> throw new UnsupportedOperationException("TBD");
> }
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).
-------------
PR: https://git.openjdk.java.net/jdk/pull/6076
