On Mon, 20 Nov 2023 16:41:08 GMT, Jonathan Gibbons <[email protected]> wrote:
>>> 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.
>
>> 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. T
>
> Since any `enum` implements `Comparable`, it is not reasonable to say that
> the order may be relied on "unexpectedly". It is an intentionally defined
> feature of the language design. And the author _can_ do something about it:
> the author can choose not to reorder constants when the order is significant.
>
> A more interesting design, back in the day, might have been to make
> `Comparable` an opt-in super type, but that's 20-20 hindsight and not the
> case.
While it is not _necessary_ to indicate that an enum implements `Comparable`,
it is permissible to state it explicitly, such as in `enum Foo implements
Comparable<Foo> { foo1, foo2 }`. which is a somewhat more linguistic way of
writing `/** The order of the constants is important. */ enum Foo { foo1, foo2
}`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16714#discussion_r1399513007
