-
Notifications
You must be signed in to change notification settings - Fork 37.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Comparable instead of dedicated implementations #25478
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ | |
* {@code true} or {@code false} first. | ||
* | ||
* @author Keith Donald | ||
* @author Eugene Rabii | ||
* @since 1.2.2 | ||
*/ | ||
@SuppressWarnings("serial") | ||
|
@@ -63,8 +64,9 @@ public BooleanComparator(boolean trueLow) { | |
|
||
|
||
@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); | ||
} | ||
|
||
|
||
|
@@ -76,7 +78,7 @@ public boolean equals(@Nullable Object other) { | |
|
||
@Override | ||
public int hashCode() { | ||
return getClass().hashCode() * (this.trueLow ? -1 : 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since |
||
return Boolean.hashCode(this.trueLow); | ||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,49 +29,44 @@ public abstract class Comparators { | |
|
||
/** | ||
* Return a {@link Comparable} adapter. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've used |
||
* @see ComparableComparator#INSTANCE | ||
*/ | ||
@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 | ||
*/ | ||
@SuppressWarnings("unchecked") | ||
public static <T> Comparator<T> nullsLow() { | ||
return NullSafeComparator.NULLS_LOW; | ||
return (Comparator<T>) Comparator.nullsLast(Comparator.naturalOrder()); | ||
} | ||
|
||
/** | ||
* Return a decorator for the given comparator which accepts | ||
* null values and sorts them lower than non-null values. | ||
* @see NullSafeComparator#NullSafeComparator(boolean) | ||
*/ | ||
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 | ||
*/ | ||
@SuppressWarnings("unchecked") | ||
public static <T> Comparator<T> nullsHigh() { | ||
return NullSafeComparator.NULLS_HIGH; | ||
return (Comparator<T>) Comparator.nullsFirst(Comparator.naturalOrder()); | ||
} | ||
|
||
/** | ||
* Return a decorator for the given comparator which accepts | ||
* null values and sorts them higher than non-null values. | ||
* @see NullSafeComparator#NullSafeComparator(boolean) | ||
*/ | ||
public static <T> Comparator<T> nullsHigh(Comparator<T> comparator) { | ||
return new NullSafeComparator<>(comparator, false); | ||
return Comparator.nullsFirst(comparator); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,34 +29,35 @@ | |
* @author Keith Donald | ||
* @author Chris Beams | ||
* @author Phillip Webb | ||
* @author Eugene Rabii | ||
*/ | ||
class BooleanComparatorTests { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tests have changed because the documentation of |
||
@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); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some time ago I had an answer about this here : https://stackoverflow.com/questions/55598277/is-there-a-useful-difference-between-p-q-and-p-q-for-booleans/55601399#55601399 and it turns out that the performance of XOR vs plain
!=
is same. This also makes it far easier to read