RFR: 8356995: Provide default methods min(T, T) and max(T, T) in Comparator interface [v2]
Raffaello Giulietti
rgiulietti at openjdk.org
Wed May 21 11:57:56 UTC 2025
On Wed, 21 May 2025 09:09:15 GMT, Tagir F. Valeev <tvaleev 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.
>
> Tagir F. Valeev has updated the pull request incrementally with one additional commit since the last revision:
>
> Return first argument in case of tie (to be consistent with BinaryOperator); junit tests
A couple of nits.
src/java.base/share/classes/java/util/Comparator.java line 200:
> 198: * @return the larger of {@code a} and {@code b} according to this comparator.
> 199: * @throws ClassCastException if the collection contains elements that are
> 200: * not <i>mutually comparable</i> (for example, strings and
`<em>` is more intent revealing.
Suggestion:
* not <em>mutually comparable</em> (for example, strings and
test/jdk/java/util/Comparator/MinMaxTest.java line 39:
> 37: public class MinMaxTest {
> 38: @Test
> 39: public void testMin() {
According to [this](https://junit.org/junit5/docs/snapshot/user-guide/#writing-tests-classes-and-methods), test methods do not need to be `public` (but cannot be `private`).
Suggestion:
void testMin() {
-------------
PR Review: https://git.openjdk.org/jdk/pull/25297#pullrequestreview-2857412002
PR Review Comment: https://git.openjdk.org/jdk/pull/25297#discussion_r2100089152
PR Review Comment: https://git.openjdk.org/jdk/pull/25297#discussion_r2100089409
More information about the core-libs-dev
mailing list