I forgot: +1 on the change itself. Hannes
> Am 27.01.2020 um 16:20 schrieb Hannes Wallnöfer > <[email protected]>: > > 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 <[email protected]>: >> >> 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/ >>> >> >
