On Fri, 17 Nov 2023 22:07:39 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> A different solution, if one is really needed, would be a regression test to >> verify the expected order. > > Yet another alternate suggestion would be to use something like > > > assert List.of(AccessLevel.values()) > .equals(List.of(PRIVATE, PACKAGE, PROTECTED, PUBLIC) > > > although I still think it is paranoid (and non-standard) to assert the > order of enum members for any enum that is used as `Comparable` > How much is the order actually relied on? Clients of `doclint.Env.AccessKind` use `compareTo()`, which is effectively defined through `ordinal()`. > A different solution, if one is really needed, would be a regression test to > verify the expected order. There are such tests already. For example, DocLintReferencesTest. > Yet another alternate suggestion would be to use something like > > ``` > assert List.of(AccessLevel.values()) > .equals(List.of(PRIVATE, PACKAGE, PROTECTED, PUBLIC) > ``` > This is certainly shorter. However, my IDE no longer flags that assert statement as "always true". > I still think it is paranoid (and non-standard) to assert the order of enum > members for any enum that is used as `Comparable` For better or worse, every enum exposes its constants' declaration order. That order may unexpectedly become relied upon elsewhere, and the author of the enum cannot do anything about it. To me, this is still a gotcha moment. When I found that doclint.Env.AccessKind.compareTo was used, my initial reaction (after Yuck!) was to introduce explicit construction AccessKind(int level) and boolean instance methods, such as lessLimiting(AccessLevel), equallyLimiting(AccessLevel) and moreLimiting(AccessLevel), that would compare int levels that the constants were constructed with. But on second thought, it felt like working against the language, which gives us all these features for free. So I decided to embrace that part of enums design and aid the next person who will look at this code, by adding a test, a comment or an assertion. There were already a few tests, so I decided not to add more. An assertion and a comment together are better than just an assertion, which is better than just a comment. Assertion is a checked comment: it jumps out at you, it fails fast, and works nicely with a sufficiently smart IDE. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16714#discussion_r1399250062
