Skip to content

Commit

Permalink
Document that ComparisonChain is mostly obsolete.
Browse files Browse the repository at this point in the history
If you are able to use the Java 8 API, you should almost certainly be using the `Comparator` methods introduced there. They are less error-prone and more succinct. In particular they avoid slip-ups like `compare(a.foo(), a.foo())` or `compare(a.foo(), b.bar())`.

RELNOTES=Documented that `ComparisonChain` is mostly obsolete.
PiperOrigin-RevId: 475862634
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Sep 21, 2022
1 parent 2e0f798 commit e452209
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 1 deletion.
100 changes: 100 additions & 0 deletions guava-tests/test/com/google/common/collect/ComparisonChainTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@

package com.google.common.collect;

import static com.google.common.base.MoreObjects.toStringHelper;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static java.lang.Integer.signum;
import static java.util.Comparator.comparing;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.nullsLast;

import com.google.common.annotations.GwtCompatible;
import com.google.common.primitives.Booleans;
import java.util.Comparator;
import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Unit test for {@link ComparisonChain}.
Expand Down Expand Up @@ -118,4 +127,95 @@ public void testCompareTrueFirst() {
assertThat(ComparisonChain.start().compareTrueFirst(false, true).result()).isGreaterThan(0);
assertThat(ComparisonChain.start().compareTrueFirst(false, false).result()).isEqualTo(0);
}

enum TriState {
FALSE,
MAYBE,
TRUE,
}

static class Foo {
private final String aString;
private final int anInt;
private final @Nullable TriState anEnum;

Foo(String aString, int anInt, @Nullable TriState anEnum) {
this.aString = aString;
this.anInt = anInt;
this.anEnum = anEnum;
}

@Override
public String toString() {
return toStringHelper(this)
.add("aString", aString)
.add("anInt", anInt)
.add("anEnum", anEnum)
.toString();
}
}

/** Validates that the Comparator equivalent we document is correct. */
public void testComparatorEquivalent() {
Comparator<Foo> comparatorUsingComparisonChain =
(a, b) ->
ComparisonChain.start()
.compare(a.aString, b.aString)
.compare(a.anInt, b.anInt)
.compare(a.anEnum, b.anEnum, Ordering.natural().nullsLast())
.result();
Comparator<Foo> comparatorUsingComparatorMethods =
comparing((Foo foo) -> foo.aString)
.thenComparing(foo -> foo.anInt)
.thenComparing(foo -> foo.anEnum, nullsLast(naturalOrder()));
ImmutableList<Foo> instances =
ImmutableList.of(
new Foo("a", 1, TriState.TRUE),
new Foo("a", 2, TriState.TRUE),
new Foo("b", 1, TriState.FALSE),
new Foo("b", 1, TriState.TRUE),
new Foo("b", 1, null));
for (Foo a : instances) {
for (Foo b : instances) {
int comparedUsingComparisonChain = signum(comparatorUsingComparisonChain.compare(a, b));
int comparedUsingComparatorMethods = signum(comparatorUsingComparatorMethods.compare(a, b));
assertWithMessage("%s vs %s", a, b)
.that(comparedUsingComparatorMethods)
.isEqualTo(comparedUsingComparisonChain);
}
}
}

static class Bar {
private final boolean isBaz;

Bar(boolean isBaz) {
this.isBaz = isBaz;
}

boolean isBaz() {
return isBaz;
}
}

/**
* Validates that {@link Booleans#trueFirst()} and {@link Booleans#falseFirst()} can be used with
* {@link Comparator} when replacing {@link ComparisonChain#compareTrueFirst} and {@link
* ComparisonChain#compareFalseFirst}, as we document.
*/
public void testTrueFirstFalseFirst() {
Bar trueBar = new Bar(true);
Bar falseBar = new Bar(false);

assertThat(ComparisonChain.start().compareTrueFirst(trueBar.isBaz(), falseBar.isBaz()).result())
.isLessThan(0);
Comparator<Bar> trueFirstComparator = comparing(Bar::isBaz, Booleans.trueFirst());
assertThat(trueFirstComparator.compare(trueBar, falseBar)).isLessThan(0);

assertThat(
ComparisonChain.start().compareFalseFirst(falseBar.isBaz(), trueBar.isBaz()).result())
.isLessThan(0);
Comparator<Bar> falseFirstComparator = comparing(Bar::isBaz, Booleans.falseFirst());
assertThat(falseFirstComparator.compare(falseBar, trueBar)).isLessThan(0);
}
}
48 changes: 47 additions & 1 deletion guava/src/com/google/common/collect/ComparisonChain.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* A utility for performing a chained comparison statement. For example:
* A utility for performing a chained comparison statement. <b>Note:</b> Java 8+ users should
* generally prefer the methods in {@link Comparator}; see <a href="#java8">below</a>.
*
* <p>Example usage of {@code ComparisonChain}:
*
* <pre>{@code
* public int compareTo(Foo that) {
Expand Down Expand Up @@ -52,6 +55,37 @@
* "https://github.com/google/guava/wiki/CommonObjectUtilitiesExplained#comparecompareto">{@code
* ComparisonChain}</a>.
*
* <h4 id="java8">Java 8+ equivalents</h4>
*
* If you are using Java version 8 or greater, you should generally use the static methods in {@link
* Comparator} instead of {@code ComparisonChain}. The example above can be implemented like this:
*
* <pre>{@code
* import static java.util.Comparator.comparing;
* import static java.util.Comparator.nullsLast;
* import static java.util.Comparator.naturalOrder;
*
* ...
* private static final Comparator<Foo> COMPARATOR =
* comparing((Foo foo) -> foo.aString)
* .thenComparing(foo -> foo.anInt)
* .thenComparing(foo -> foo.anEnum, nullsLast(naturalOrder()));}
*
* {@code @Override}{@code
* public int compareTo(Foo that) {
* return COMPARATOR.compare(this, that);
* }
* }</pre>
*
* <p>With method references it is more succinct: {@code comparing(Foo::aString)} for example.
*
* <p>Using {@link Comparator} avoids certain types of bugs, for example when you meant to write
* {@code .compare(a.foo, b.foo)} but you actually wrote {@code .compare(a.foo, a.foo)} or {@code
* .compare(a.foo, b.bar)}. {@code ComparisonChain} also has a potential performance problem that
* {@code Comparator} doesn't: it evaluates all the parameters of all the {@code .compare} calls,
* even when the result of the comparison is already known from previous {@code .compare} calls.
* That can be expensive.
*
* @author Mark Davis
* @author Kevin Bourrillion
* @since 2.0
Expand Down Expand Up @@ -243,6 +277,12 @@ public final ComparisonChain compare(Boolean left, Boolean right) {
* Compares two {@code boolean} values, considering {@code true} to be less than {@code false},
* <i>if</i> the result of this comparison chain has not already been determined.
*
* <p>Java 8+ users: you can get the equivalent from {@link Booleans#trueFirst()}. For example:
*
* <pre>
* Comparator.comparing(Foo::isBar, {@link Booleans#trueFirst()})
* </pre>
*
* @since 12.0
*/
public abstract ComparisonChain compareTrueFirst(boolean left, boolean right);
Expand All @@ -251,6 +291,12 @@ public final ComparisonChain compare(Boolean left, Boolean right) {
* Compares two {@code boolean} values, considering {@code false} to be less than {@code true},
* <i>if</i> the result of this comparison chain has not already been determined.
*
* <p>Java 8+ users: you can get the equivalent from {@link Booleans#falseFirst()}. For example:
*
* <pre>
* Comparator.comparing(Foo::isBar, {@link Booleans#falseFirst()})
* </pre>
*
* @since 12.0 (present as {@code compare} since 2.0)
*/
public abstract ComparisonChain compareFalseFirst(boolean left, boolean right);
Expand Down

0 comments on commit e452209

Please sign in to comment.