I see you renamed some other methods as well. Looks good! Hannes
> Am 18.02.2020 um 16:13 schrieb Pavel Rappo <[email protected]>: > > Hi Hannes, > > I've updated the webrev: > > http://cr.openjdk.java.net/~prappo/8239243/webrev.01/ > > Thanks! > >> On 18 Feb 2020, at 09:52, Hannes Wallnöfer <[email protected]> >> wrote: >> >> Hi Pavel, >> >> +1 for the conditional index building, and the IndexBuilder cleanup is a >> major improvement in every respect. >> >> Nitpicking here, but two naming suggestions: >> >> - „index“ can be both verb and noun, so renaming the „index“ method to >> „indexElements“ might make its meaning a bit clearer and puts it in line >> with the other index* methods >> >> - since firstCharacter(String) doesn’t always return the first character >> maybe something like „indexCharacter“ or „keyCharacter“ would be more >> fitting? >> >> Hannes >> >> >>> Am 17.02.2020 um 16:35 schrieb Pavel Rappo <[email protected]>: >>> >>> Hello, >>> >>> Please review the change for >>> https://bugs.openjdk.java.net/browse/JDK-8239243: >>> >>> http://cr.openjdk.java.net/~prappo/8239243/webrev.00/ >>> >>> I noticed that the index structure, used in Index Page an others, is created >>> unconditionally (i.e. regardless of the presence of the "-noindex" >>> command-line >>> option). Since populating that index doesn't seem to have any side-effects, >>> this >>> will unnecessarily consume resources of documentation generation process >>> when no >>> index is required. Thus, moving it under the relevant if-statement should be >>> safe and the right thing to do. I've also cleaned up the IndexBuilder class >>> a bit. >>> >>> In case you wonder, when generating the API docs for the JDK 15, it takes >>> about >>> 1 sec. (one second) to create that structure. The number of instances of >>> Element >>> in that Map<Character, SortedSet<Element>> is roughly 50,000 (fifty >>> thousand). >>> >>> -Pavel >>> >> >
