On Mon, 12 Oct 2020 22:36:51 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> Inspired by other recent work to cleanup index generation, this change is > primarily about cleanup for > `AbstractIndexWriter`, `SingleIndexWriter`, `SplitIndexWriter` to reduce code > duplication and to reduce the 3 class to > just 1 that can handle either single index files or split index files. As > part of this cleanup, some anomalies were > uncovered in the naming of annotation types, enum constants and records, > which have also been addressed. > The changes involved moving/merging code from `SingleIndexWriter` and > `SplitIndexWriter` in to `AbstractIndexWriter`, > which is now no longer abstract and is thus renamed to just `IndexWriter`. > Regrettably, the rename is not properly > tracked. Nice work! src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/IndexWriter.java line 132: > 130: Content mainContent = new ContentBuilder(); > 131: List<Character> allFirstCharacters = new > ArrayList<>(mainIndex.getFirstCharacters()); > 132: allFirstCharacters.sort(Comparator.naturalOrder()); It seems a bit redundant to retrieve and sort the `allFirstCharacters` list in each invocation of `generateIndexFile`, maybe pass it as an argument from the `generate(HtmlConfiguration)` method? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/IndexWriter.java line 87: > 85: IndexBuilder mainIndex = configuration.mainIndex; > 86: List<Character> firstCharacters = mainIndex.getFirstCharacters(); > 87: firstCharacters.sort(Comparator.naturalOrder()); Is this sorting step necessary? Aren't the characters returned by `IndexBuilder#getFirstCharacters()` already guaranteed to be in natural order? ------------- Marked as reviewed by hannesw (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/621
