Skip to content

Commit

Permalink
Fix Comparators.nullsLow and Comporators.nullsHigh behavior
Browse files Browse the repository at this point in the history
Commit 33454a4 introduced a regression in Comparators.nullsLow() and
Comporators.nullsHigh().

This commit updates the code so that nullsLow() sorts null values lower
than non-null values and nullsHigh sorts null values higher than
non-null values.

See gh-25478
Closes gh-31808
  • Loading branch information
mathieu-amblard authored and sbrannen committed Dec 11, 2023
1 parent d75a7c3 commit a013840
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static <T> Comparator<T> comparable() {
/**
* Return a {@link Comparable} adapter which accepts
* null values and sorts them lower than non-null values.
* @see Comparator#nullsLast(Comparator)
* @see Comparator#nullsFirst(Comparator)
*/
public static <T> Comparator<T> nullsLow() {
return nullsLow(comparable());
Expand All @@ -48,16 +48,16 @@ public static <T> Comparator<T> nullsLow() {
/**
* Return a decorator for the given comparator which accepts
* null values and sorts them lower than non-null values.
* @see Comparator#nullsLast(Comparator)
* @see Comparator#nullsFirst(Comparator)
*/
public static <T> Comparator<T> nullsLow(Comparator<T> comparator) {
return Comparator.nullsLast(comparator);
return Comparator.nullsFirst(comparator);
}

/**
* Return a {@link Comparable} adapter which accepts
* null values and sorts them higher than non-null values.
* @see Comparator#nullsFirst(Comparator)
* @see Comparator#nullsLast(Comparator)
*/
public static <T> Comparator<T> nullsHigh() {
return nullsHigh(comparable());
Expand All @@ -66,10 +66,10 @@ public static <T> Comparator<T> nullsHigh() {
/**
* Return a decorator for the given comparator which accepts
* null values and sorts them higher than non-null values.
* @see Comparator#nullsFirst(Comparator)
* @see Comparator#nullsLast(Comparator)
*/
public static <T> Comparator<T> nullsHigh(Comparator<T> comparator) {
return Comparator.nullsFirst(comparator);
return Comparator.nullsLast(comparator);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.util.comparator;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

/**
* Tests for {@link Comparators}
*/
class ComparatorsTest {

@Test
void shouldCompareWithNullsLow() {
assertThat(Comparators.nullsLow().compare(null, "boo")).isNegative();
assertThat(Comparators.nullsLow().compare(null, null)).isZero();
}

@Test
void shouldCompareWithNullsLowAndComparator() {
assertThat(Comparators.nullsLow(String::compareTo).compare(null, "boo")).isNegative();
assertThat(Comparators.nullsLow(String::compareTo).compare(null, null)).isZero();
}

@Test
void shouldCompareWithNullsHigh() {
assertThat(Comparators.nullsHigh().compare(null, "boo")).isPositive();
assertThat(Comparators.nullsHigh().compare(null, null)).isZero();
}

@Test
void shouldCompareWithNullsHighAndComparator() {
assertThat(Comparators.nullsHigh(String::compareTo).compare(null, "boo")).isPositive();
assertThat(Comparators.nullsHigh(String::compareTo).compare(null, null)).isZero();
}
}

0 comments on commit a013840

Please sign in to comment.