From 49cafe07ede1add46c2e3885f79b7bd3b92c557e Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 25 Aug 2023 14:32:09 +0200 Subject: [PATCH] Polish "Use Comparable instead of dedicated implementations" See gh-25478 --- .../util/comparator/ComparableComparator.java | 7 +++---- .../util/comparator/Comparators.java | 13 ++++++++----- .../util/comparator/NullSafeComparator.java | 7 +++++-- .../converter/ConvertingComparatorTests.java | 16 ++++++++-------- .../util/ConcurrentReferenceHashMapTests.java | 8 +++----- .../util/comparator/BooleanComparatorTests.java | 2 +- .../comparator/ComparableComparatorTests.java | 1 + .../util/comparator/NullSafeComparatorTests.java | 1 + 8 files changed, 30 insertions(+), 25 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/comparator/ComparableComparator.java b/spring-core/src/main/java/org/springframework/util/comparator/ComparableComparator.java index 27d36a38eb7..48a11892833 100644 --- a/spring-core/src/main/java/org/springframework/util/comparator/ComparableComparator.java +++ b/spring-core/src/main/java/org/springframework/util/comparator/ComparableComparator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,18 +23,17 @@ import java.util.Comparator; * Mainly for internal use in other Comparators, when supposed * to work on Comparables. * - * @deprecated use jdk-8 Comparator::naturalOrder * @author Keith Donald * @since 1.2.2 * @param the type of comparable objects that may be compared by this comparator * @see Comparable + * @deprecated as of 6.1 in favor of {@link Comparator#naturalOrder()} */ -@Deprecated +@Deprecated(since = "6.1") public class ComparableComparator> implements Comparator { /** * A shared instance of this default comparator. - * * @see Comparators#comparable() */ @SuppressWarnings("rawtypes") diff --git a/spring-core/src/main/java/org/springframework/util/comparator/Comparators.java b/spring-core/src/main/java/org/springframework/util/comparator/Comparators.java index 36e7076045f..607f82371dd 100644 --- a/spring-core/src/main/java/org/springframework/util/comparator/Comparators.java +++ b/spring-core/src/main/java/org/springframework/util/comparator/Comparators.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ public abstract class Comparators { /** * Return a {@link Comparable} adapter. + * @see Comparator#naturalOrder() */ @SuppressWarnings("unchecked") public static Comparator comparable() { @@ -38,15 +39,16 @@ public abstract class Comparators { /** * Return a {@link Comparable} adapter which accepts * null values and sorts them lower than non-null values. + * @see Comparator#nullsLast(Comparator) */ - @SuppressWarnings("unchecked") public static Comparator nullsLow() { - return (Comparator) Comparator.nullsLast(Comparator.naturalOrder()); + return nullsLow(comparable()); } /** * Return a decorator for the given comparator which accepts * null values and sorts them lower than non-null values. + * @see Comparator#nullsLast(Comparator) */ public static Comparator nullsLow(Comparator comparator) { return Comparator.nullsLast(comparator); @@ -55,15 +57,16 @@ public abstract class Comparators { /** * Return a {@link Comparable} adapter which accepts * null values and sorts them higher than non-null values. + * @see Comparator#nullsFirst(Comparator) */ - @SuppressWarnings("unchecked") public static Comparator nullsHigh() { - return (Comparator) Comparator.nullsFirst(Comparator.naturalOrder()); + return nullsHigh(comparable()); } /** * Return a decorator for the given comparator which accepts * null values and sorts them higher than non-null values. + * @see Comparator#nullsFirst(Comparator) */ public static Comparator nullsHigh(Comparator comparator) { return Comparator.nullsFirst(comparator); diff --git a/spring-core/src/main/java/org/springframework/util/comparator/NullSafeComparator.java b/spring-core/src/main/java/org/springframework/util/comparator/NullSafeComparator.java index 4157f1cc80e..77dce760827 100644 --- a/spring-core/src/main/java/org/springframework/util/comparator/NullSafeComparator.java +++ b/spring-core/src/main/java/org/springframework/util/comparator/NullSafeComparator.java @@ -30,7 +30,10 @@ import org.springframework.util.Assert; * @since 1.2.2 * @param the type of objects that may be compared by this comparator * @see Comparable + * @see Comparators + * @deprecated as of 6.1 in favor of {@link Comparator#nullsLast} and {@link Comparator#nullsFirst} */ +@Deprecated(since = "6.1") public class NullSafeComparator implements Comparator { /** @@ -49,6 +52,7 @@ public class NullSafeComparator implements Comparator { @SuppressWarnings("rawtypes") public static final NullSafeComparator NULLS_HIGH = new NullSafeComparator<>(false); + private final Comparator nonNullComparator; private final boolean nullsLow; @@ -68,9 +72,8 @@ public class NullSafeComparator implements Comparator { * @see #NULLS_LOW * @see #NULLS_HIGH */ - @SuppressWarnings("unchecked") private NullSafeComparator(boolean nullsLow) { - this.nonNullComparator = (Comparator) Comparator.naturalOrder(); + this.nonNullComparator = Comparators.comparable(); this.nullsLow = nullsLow; } diff --git a/spring-core/src/test/java/org/springframework/core/convert/converter/ConvertingComparatorTests.java b/spring-core/src/test/java/org/springframework/core/convert/converter/ConvertingComparatorTests.java index b7903d06975..67f71684286 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/converter/ConvertingComparatorTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/converter/ConvertingComparatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,7 +26,7 @@ import org.junit.jupiter.api.Test; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.util.comparator.ComparableComparator; +import org.springframework.util.comparator.Comparators; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -95,17 +95,17 @@ class ConvertingComparatorTests { } @Test - void shouldGetMapEntryKeys() throws Exception { + void shouldGetMapEntryKeys() { ArrayList> list = createReverseOrderMapEntryList(); - Comparator> comparator = ConvertingComparator.mapEntryKeys(new ComparableComparator()); + Comparator> comparator = ConvertingComparator.mapEntryKeys(Comparators.comparable()); list.sort(comparator); assertThat(list.get(0).getKey()).isEqualTo("a"); } @Test - void shouldGetMapEntryValues() throws Exception { + void shouldGetMapEntryValues() { ArrayList> list = createReverseOrderMapEntryList(); - Comparator> comparator = ConvertingComparator.mapEntryValues(new ComparableComparator()); + Comparator> comparator = ConvertingComparator.mapEntryValues(Comparators.comparable()); list.sort(comparator); assertThat(list.get(0).getValue()).isEqualTo(1); } @@ -130,7 +130,7 @@ class ConvertingComparatorTests { } - private static class TestComparator extends ComparableComparator { + private static class TestComparator implements Comparator { private boolean called; @@ -139,7 +139,7 @@ class ConvertingComparatorTests { assertThat(o1).isInstanceOf(Integer.class); assertThat(o2).isInstanceOf(Integer.class); this.called = true; - return super.compare(o1, o2); + return Comparators.comparable().compare(o1, o2); } public void assertCalled() { diff --git a/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java b/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java index df1ef39989a..75b084713cc 100644 --- a/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java +++ b/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,8 +36,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.ConcurrentReferenceHashMap.Entry; import org.springframework.util.ConcurrentReferenceHashMap.Reference; import org.springframework.util.ConcurrentReferenceHashMap.Restructure; -import org.springframework.util.comparator.ComparableComparator; -import org.springframework.util.comparator.NullSafeComparator; +import org.springframework.util.comparator.Comparators; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -51,8 +50,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; */ class ConcurrentReferenceHashMapTests { - private static final Comparator NULL_SAFE_STRING_SORT = new NullSafeComparator<>( - new ComparableComparator(), true); + private static final Comparator NULL_SAFE_STRING_SORT = Comparators.nullsLow(); private TestWeakConcurrentCache map = new TestWeakConcurrentCache<>(); diff --git a/spring-core/src/test/java/org/springframework/util/comparator/BooleanComparatorTests.java b/spring-core/src/test/java/org/springframework/util/comparator/BooleanComparatorTests.java index 2022825b5ed..bb3b766160d 100644 --- a/spring-core/src/test/java/org/springframework/util/comparator/BooleanComparatorTests.java +++ b/spring-core/src/test/java/org/springframework/util/comparator/BooleanComparatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-core/src/test/java/org/springframework/util/comparator/ComparableComparatorTests.java b/spring-core/src/test/java/org/springframework/util/comparator/ComparableComparatorTests.java index 5171625fa2a..9f9235d8462 100644 --- a/spring-core/src/test/java/org/springframework/util/comparator/ComparableComparatorTests.java +++ b/spring-core/src/test/java/org/springframework/util/comparator/ComparableComparatorTests.java @@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * @author Chris Beams * @author Phillip Webb */ +@Deprecated class ComparableComparatorTests { @Test diff --git a/spring-core/src/test/java/org/springframework/util/comparator/NullSafeComparatorTests.java b/spring-core/src/test/java/org/springframework/util/comparator/NullSafeComparatorTests.java index 83a41b1154e..03c20ff6c5a 100644 --- a/spring-core/src/test/java/org/springframework/util/comparator/NullSafeComparatorTests.java +++ b/spring-core/src/test/java/org/springframework/util/comparator/NullSafeComparatorTests.java @@ -29,6 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Chris Beams * @author Phillip Webb */ +@Deprecated class NullSafeComparatorTests { @SuppressWarnings("unchecked")