On Wed, 10 Nov 2021 11:40:28 GMT, Anthony Vanelverdinghe <d...@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
