RFR: JDK-8237845 Encapsulate doclet options

[Jonathan Gibbons]

Yeah, I was very aware of the weirdness when I submitted the review, 
particularly for the names that are more like verbs than nouns or 
adjectives.

My short-term justification is, as Hannes noted, they're always invoked 
on an "options" object.  Further out, I'm open to better naming. One 
thing to note is that this changeset moves us towards a more consistent 
design that is more amenable to IDE-based updates (i.e. "rename method" 
refactoring) down the line.

We can also try and come up with a scheme for naming option accessor 
methods that covers both tool options and all the doclet options.

-- Jon


On 1/27/20 7:22 AM, Hannes Wallnöfer wrote:
> I forgot: +1 on the change itself.
>
> Hannes
>
>> Am 27.01.2020 um 16:20 schrieb Hannes Wallnöfer <hannes.wallnoefer at oracle.com>:
>>
>> I agree some of these method names taken on their own are slightly deceptive. But given their mostly ()boolean  signatures and the fact that they’re always invoked on ‚options‘ or similar make their purpose clear enough without having to look at their implementation or doc comments.
>>
>> Given that neither get*, is*, or has* prefix is really great for this use case I think keeping these names and maybe adding a comment as suggested by Pavel is the best option.
>>
>> Hannes
>>
>>
>>> Am 27.01.2020 um 14:38 schrieb Pavel Rappo <pavel.rappo at oracle.com>:
>>>
>>> I remember we had an offline conversation on whether we should use shorter
>>> method names for accessing stuff [ stuff() ], or more familiar ones
>>> [ getStuff()/hasStuff()/isStuff()/etc. ].
>>> I remember saying that I had no strong opinion about it. Not thank I'm
>>> rethinking it, but I can now see how some of the accessor methods look weird.
>>> Unsurprisingly, those are the ones whose name starts with a verb. For example,
>>>
>>>    copyDocfileSubdirs(),
>>>    createIndex(),
>>>    createOverview(),
>>>    linkSource(),
>>>    showAuthor(),
>>>    showVersion(),
>>>    splitIndex(),
>>>    summarizeOverriddenMethods(), etc.
>>>
>>> Naming is hard, no doubt about it. While giving something a name, that name
>>> should not be expected to be the sole guide on how to use that something.
>>> A name should be a good mnemonic.
>>>
>>> We could put a top-level comment in both BaseOptions and HtmlOptions. Something
>>> to the effect that "unless otherwise stated, the methods below do not have any
>>> behavior beyond simply accessing stuff".
>>>
>>> Otherwise, looks good.
>>>
>>> -Pavel
>>>
>>>> On 25 Jan 2020, at 00:33, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
>>>>
>>>> Please review a conceptually simple review to encapsulate the fields holding the values of doclet options, in the BaseOptions and HtmlOptions classes.
>>>>
>>>> The encapsulation was done with an IDE. The fields are all changed to private access; the access methods are package-private by default, and public when necessary, although in general, there is no problem if it is determined they should all have public access.  This is the same policy as was done for ToolOptions, currently in review.
>>>>
>>>> Some fields need public "setters". These were added manually.
>>>>
>>>> In a few places, code was simplified manually, joining short lines, or replacing `configuration.getOptions()` with just `options`.
>>>>
>>>> This is intended to be the last in the current round of option-code cleanup, for now, although I think there is scope for future work to rationalize the names of the fields and corresponding accessor methods.
>>>>
>>>> -- Jon
>>>>
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8237845
>>>> Webrev: http://cr.openjdk.java.net/~jjg/8237845/webrev.00/
>>>>