RFR: 8356995: Provide default methods min(T, T) and max(T, T) in Comparator interface
Tagir F. Valeev
tvaleev at openjdk.org
Wed May 21 09:13:55 UTC 2025
On Mon, 19 May 2025 17:06:17 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:
>> Implementation of Comparator.min and Comparator.max methods. Preliminary discussion is in this thread:
>> https://mail.openjdk.org/pipermail/core-libs-dev/2025-May/145638.html
>> The specification is mostly composed of Math.min/max and Collections.min/max specifications.
>>
>> The methods are quite trivial, so I don't think we need more extensive testing (e.g., using different comparators). But if you have ideas of new useful tests, I'll gladly add them.
>>
>> I'm not sure whether we should specify exactly the behavior in case if the comparator returns 0. I feel that it could be a useful invariant that `Comparator.min(a, b)` and `Comparator.max(a, b)` always return different argument, partitioning the set of {a, b} objects (even if they are equal). But I'm open to suggestions here.
>
> @amaembo Could you please switch the test class to make use of [JUnit](https://junit.org/junit5/)? For new tests we now prefer JUnit over TestNG.
> Thanks!
@rgiulietti I tried to mimic the nearby tests which use testng. Converted to junit 5.
@archiecobbs
> IMHO it makes sense. It's the min/max analog to a stable sort
On the second thought, there is a consistency argument. We already have BinaryOperator.minBy and BinaryOperator.maxBy, which always return the first argument in case of tie (though this is not specified, probably it should be?). So it looks like it will be better to have both APIs consistent. One more point is that Guava's [`Ordering`](https://guava.dev/releases/snapshot-jre/api/docs/com/google/common/collect/Ordering.html#max(E,E)) (which extends `Comparator`) also returns the first argument in case of tie, so if we do the same, `Ordering` will not violate the `Comparator` contract. Thus I've changed the implementation to be in sync with both BinaryOperator and Ordering. This also addresses the @RogerRiggs comment: now implementations use `>=` and `<=`.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25297#issuecomment-2897204193
More information about the core-libs-dev
mailing list