RFR: 8310241: OffsetDateTime compareTo redundant computation [v2]

Naoto Sato naoto at openjdk.org
Wed Jun 28 19:38:01 UTC 2023


On Wed, 28 Jun 2023 17:46:51 GMT, Roger Riggs <rriggs at 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 `compareTo` to do the comparison of `LocalDateTime` exactly once, if it is needed.
>> 
>> This case is NOT covered by an existing test in test/jdk/java/time/tck/java/time/TCKOffsetDateTime.java
>
> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reopen to work on an improved fix
>   Add a test for the specific condition being optimized, the test was missing in the original.

test/jdk/java/time/tck/java/time/TCKOffsetDateTime.java line 1384:

> 1382:         assertEquals(b.compareTo(b) == 0, true);
> 1383:         assertEquals(a.toInstant().compareTo(b.toInstant()) == 0, true);
> 1384:         assertEquals(OffsetDateTime.timeLineOrder().compare(a,b) == 0, true);

Could be in a separate test case, as this method tests `compareTo` method?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14618#discussion_r1245676546


More information about the core-libs-dev mailing list