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 <[email protected]>
> 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/
>
