On Mon, 26 Jun 2023 18:28:47 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Remove a redundant comparison in java.time `OffsetDateTime.compareTo()`.
>> If the `compareInstant` utility method returns 0 (equal), it compares the
>> `LocalDateTime`.
>> However, `compareInstant` has already done that comparison; if it found
>> equal, the `compareTo` method unnecessarily does it again.
>> The code is refactored in `compareInstant` to do the comparison of
>> `LocalDateTime` exactly once, if it is needed.
>>
>> This case is covered by existing tests in
>> test/jdk/java/time/tck/java/time/TCKOffsetDateTime.java
>
> src/java.base/share/classes/java/time/OffsetDateTime.java line 173:
>
>> 171: * to distinguish them. This step is needed to make the ordering
>> 172: * consistent with {@code equals()}.
>> 173: *
>
> This method is also called from the public `timeLineOrder()` method. Could
> this change cause a different order?
Your are right; I missed that subtlety. With that in mind I don't think the
performance impact is sufficient to duplicate more code.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14618#discussion_r1242723719
