Merge pull request #25478 from wind57

* pr/25478:
  Polish "Use Comparable instead of dedicated implementations"
  Use Comparable instead of dedicated implementations

Closes gh-25478
This commit is contained in:
Stephane Nicoll 2023-08-25 14:47:53 +02:00
commit 178cb429a8
9 changed files with 46 additions and 49 deletions

View File

@ -26,6 +26,7 @@ import org.springframework.lang.Nullable;
* {@code true} or {@code false} first.
*
* @author Keith Donald
* @author Eugene Rabii
* @since 1.2.2
*/
@SuppressWarnings("serial")
@ -63,8 +64,9 @@ public class BooleanComparator implements Comparator<Boolean>, Serializable {
@Override
public int compare(Boolean v1, Boolean v2) {
return (v1 ^ v2) ? ((v1 ^ this.trueLow) ? 1 : -1) : 0;
public int compare(Boolean left, Boolean right) {
int multiplier = this.trueLow ? -1 : 1;
return multiplier * Boolean.compare(left, right);
}
@ -75,7 +77,7 @@ public class BooleanComparator implements Comparator<Boolean>, Serializable {
@Override
public int hashCode() {
return getClass().hashCode() * (this.trueLow ? -1 : 1);
return Boolean.hashCode(this.trueLow);
}
@Override

View File

@ -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.
@ -27,7 +27,9 @@ import java.util.Comparator;
* @since 1.2.2
* @param <T> 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(since = "6.1")
public class ComparableComparator<T extends Comparable<T>> implements Comparator<T> {
/**

View File

@ -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,49 +29,47 @@ public abstract class Comparators {
/**
* Return a {@link Comparable} adapter.
* @see ComparableComparator#INSTANCE
* @see Comparator#naturalOrder()
*/
@SuppressWarnings("unchecked")
public static <T> Comparator<T> comparable() {
return ComparableComparator.INSTANCE;
return (Comparator<T>) Comparator.naturalOrder();
}
/**
* Return a {@link Comparable} adapter which accepts
* null values and sorts them lower than non-null values.
* @see NullSafeComparator#NULLS_LOW
* @see Comparator#nullsLast(Comparator)
*/
@SuppressWarnings("unchecked")
public static <T> Comparator<T> nullsLow() {
return NullSafeComparator.NULLS_LOW;
return nullsLow(comparable());
}
/**
* Return a decorator for the given comparator which accepts
* null values and sorts them lower than non-null values.
* @see NullSafeComparator#NullSafeComparator(boolean)
* @see Comparator#nullsLast(Comparator)
*/
public static <T> Comparator<T> nullsLow(Comparator<T> comparator) {
return new NullSafeComparator<>(comparator, true);
return Comparator.nullsLast(comparator);
}
/**
* Return a {@link Comparable} adapter which accepts
* null values and sorts them higher than non-null values.
* @see NullSafeComparator#NULLS_HIGH
* @see Comparator#nullsFirst(Comparator)
*/
@SuppressWarnings("unchecked")
public static <T> Comparator<T> nullsHigh() {
return NullSafeComparator.NULLS_HIGH;
return nullsHigh(comparable());
}
/**
* Return a decorator for the given comparator which accepts
* null values and sorts them higher than non-null values.
* @see NullSafeComparator#NullSafeComparator(boolean)
* @see Comparator#nullsFirst(Comparator)
*/
public static <T> Comparator<T> nullsHigh(Comparator<T> comparator) {
return new NullSafeComparator<>(comparator, false);
return Comparator.nullsFirst(comparator);
}
}

View File

@ -30,7 +30,10 @@ import org.springframework.util.Assert;
* @since 1.2.2
* @param <T> 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<T> implements Comparator<T> {
/**
@ -69,9 +72,8 @@ public class NullSafeComparator<T> implements Comparator<T> {
* @see #NULLS_LOW
* @see #NULLS_HIGH
*/
@SuppressWarnings("unchecked")
private NullSafeComparator(boolean nullsLow) {
this.nonNullComparator = ComparableComparator.INSTANCE;
this.nonNullComparator = Comparators.comparable();
this.nullsLow = nullsLow;
}
@ -92,17 +94,9 @@ public class NullSafeComparator<T> implements Comparator<T> {
@Override
public int compare(@Nullable T o1, @Nullable T o2) {
if (o1 == o2) {
return 0;
}
if (o1 == null) {
return (this.nullsLow ? -1 : 1);
}
if (o2 == null) {
return (this.nullsLow ? 1 : -1);
}
return this.nonNullComparator.compare(o1, o2);
public int compare(@Nullable T left, @Nullable T right) {
Comparator<T> comparator = this.nullsLow ? Comparator.nullsFirst(this.nonNullComparator) : Comparator.nullsLast(this.nonNullComparator);
return comparator.compare(left, right);
}
@ -115,7 +109,7 @@ public class NullSafeComparator<T> implements Comparator<T> {
@Override
public int hashCode() {
return this.nonNullComparator.hashCode() * (this.nullsLow ? -1 : 1);
return Boolean.hashCode(this.nullsLow);
}
@Override

View File

@ -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<Entry<String, Integer>> list = createReverseOrderMapEntryList();
Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryKeys(new ComparableComparator<String>());
Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryKeys(Comparators.comparable());
list.sort(comparator);
assertThat(list.get(0).getKey()).isEqualTo("a");
}
@Test
void shouldGetMapEntryValues() throws Exception {
void shouldGetMapEntryValues() {
ArrayList<Entry<String, Integer>> list = createReverseOrderMapEntryList();
Comparator<Map.Entry<String, Integer>> comparator = ConvertingComparator.mapEntryValues(new ComparableComparator<Integer>());
Comparator<Map.Entry<String, Integer>> 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<Integer> {
private static class TestComparator implements Comparator<Integer> {
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() {

View File

@ -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<? super String> NULL_SAFE_STRING_SORT = new NullSafeComparator<>(
new ComparableComparator<String>(), true);
private static final Comparator<? super String> NULL_SAFE_STRING_SORT = Comparators.nullsLow();
private TestWeakConcurrentCache<Integer, String> map = new TestWeakConcurrentCache<>();

View File

@ -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.
@ -29,34 +29,35 @@ import static org.assertj.core.api.Assertions.assertThat;
* @author Keith Donald
* @author Chris Beams
* @author Phillip Webb
* @author Eugene Rabii
*/
class BooleanComparatorTests {
@Test
void shouldCompareWithTrueLow() {
Comparator<Boolean> c = new BooleanComparator(true);
assertThat(c.compare(true, false)).isEqualTo(-1);
assertThat(c.compare(true, false)).isLessThan(0);
assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
}
@Test
void shouldCompareWithTrueHigh() {
Comparator<Boolean> c = new BooleanComparator(false);
assertThat(c.compare(true, false)).isEqualTo(1);
assertThat(c.compare(true, false)).isGreaterThan(0);
assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
}
@Test
void shouldCompareFromTrueLow() {
Comparator<Boolean> c = BooleanComparator.TRUE_LOW;
assertThat(c.compare(true, false)).isEqualTo(-1);
assertThat(c.compare(true, false)).isLessThan(0);
assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
}
@Test
void shouldCompareFromTrueHigh() {
Comparator<Boolean> c = BooleanComparator.TRUE_HIGH;
assertThat(c.compare(true, false)).isEqualTo(1);
assertThat(c.compare(true, false)).isGreaterThan(0);
assertThat(c.compare(Boolean.TRUE, Boolean.TRUE)).isEqualTo(0);
}

View File

@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
* @author Chris Beams
* @author Phillip Webb
*/
@Deprecated
class ComparableComparatorTests {
@Test

View File

@ -29,6 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* @author Chris Beams
* @author Phillip Webb
*/
@Deprecated
class NullSafeComparatorTests {
@SuppressWarnings("unchecked")