This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-collections.git
commit 4cac454dedb6072ac614bfe560c85f02d5618ea1
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Sun Mar 31 11:32:45 2024 -0400
Reimplement FixedOrderComparator#equals() and hashCode(), see also #392
---
src/changes/changes.xml | 3 +
.../comparators/FixedOrderComparator.java | 44 ++-------
.../comparators/FixedOrderComparatorTest.java | 100 +++++++++++++++++----
3 files changed, 96 insertions(+), 51 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 2982caa62..203d70d17 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -209,6 +209,9 @@
<action issue="COLLECTIONS-850" dev="ggregory" type="fix" due-to="Gary
Gregory, Anirudh Madhavan">
Tests in org.apache.commons.collections4.multimap should not depend on
map iteration order, see also #429.
</action>
+ <action issue="COLLECTIONS-850" dev="ggregory" type="fix" due-to="Gary
Gregory, Saurabh Rahate">
+ Reimplement FixedOrderComparator#equals() and hashCode(), see also #392.
+ </action>
<!-- ADD -->
<action issue="COLLECTIONS-843" dev="ggregory" type="add" due-to="Claude
Warren, Alex Herbert, Gary Gregory">
Implement Layered Bloom filter #402.
diff --git
a/src/main/java/org/apache/commons/collections4/comparators/FixedOrderComparator.java
b/src/main/java/org/apache/commons/collections4/comparators/FixedOrderComparator.java
index bd98c69e6..7c00ac38b 100644
---
a/src/main/java/org/apache/commons/collections4/comparators/FixedOrderComparator.java
+++
b/src/main/java/org/apache/commons/collections4/comparators/FixedOrderComparator.java
@@ -201,36 +201,19 @@ public class FixedOrderComparator<T> implements
Comparator<T>, Serializable {
return position1.compareTo(position2);
}
- /**
- * Returns {@code true} iff <i>that</i> Object is
- * a {@link Comparator} whose ordering is known to be
- * equivalent to mine.
- * <p>
- * This implementation returns {@code true}
- * iff {@code <i>that</i>} is a {@link FixedOrderComparator}
- * whose attributes are equal to mine.
- *
- * @param object the object to compare to
- * @return true if equal
- */
@Override
- public boolean equals(final Object object) {
- if (this == object) {
+ public boolean equals(Object obj) {
+ if (this == obj) {
return true;
}
- if (null == object) {
+ if (obj == null) {
return false;
}
- if (object.getClass().equals(this.getClass())) {
- final FixedOrderComparator<?> comp = (FixedOrderComparator<?>)
object;
- return (null == map ? null == comp.map : map.equals(comp.map)) &&
- (null == unknownObjectBehavior ? null ==
comp.unknownObjectBehavior :
- unknownObjectBehavior == comp.unknownObjectBehavior &&
- counter == comp.counter &&
- isLocked == comp.isLocked &&
- unknownObjectBehavior == comp.unknownObjectBehavior);
+ if (getClass() != obj.getClass()) {
+ return false;
}
- return false;
+ FixedOrderComparator other = (FixedOrderComparator) obj;
+ return counter == other.counter && isLocked == other.isLocked &&
Objects.equals(map, other.map) && unknownObjectBehavior ==
other.unknownObjectBehavior;
}
/**
@@ -242,20 +225,9 @@ public class FixedOrderComparator<T> implements
Comparator<T>, Serializable {
return unknownObjectBehavior;
}
- /**
- * Implement a hash code for this comparator that is consistent with
- * {@link #equals(Object) equals}.
- *
- * @return a hash code for this comparator.
- */
@Override
public int hashCode() {
- int total = 17;
- total = total*37 + map.hashCode();
- total = total*37 + (unknownObjectBehavior == null ? 0 :
unknownObjectBehavior.hashCode());
- total = total*37 + counter;
- total = total*37 + (isLocked ? 0 : 1);
- return total;
+ return Objects.hash(counter, isLocked, map, unknownObjectBehavior);
}
// Bean methods / state querying methods
diff --git
a/src/test/java/org/apache/commons/collections4/comparators/FixedOrderComparatorTest.java
b/src/test/java/org/apache/commons/collections4/comparators/FixedOrderComparatorTest.java
index a913acf8e..f8578c63c 100644
---
a/src/test/java/org/apache/commons/collections4/comparators/FixedOrderComparatorTest.java
+++
b/src/test/java/org/apache/commons/collections4/comparators/FixedOrderComparatorTest.java
@@ -28,6 +28,7 @@ import java.util.List;
import java.util.Random;
import org.apache.commons.lang3.ArrayUtils;
+import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
/**
@@ -35,6 +36,80 @@ import org.junit.jupiter.api.Test;
*/
public class FixedOrderComparatorTest extends AbstractComparatorTest<String> {
+ @Nested
+ class Equals {
+
+ @Test
+ void expectFalseWhenBothComparatorsWithDifferentItems() {
+ FixedOrderComparator<Integer> comparator1 = new
FixedOrderComparator<>(1, 2, 3);
+ FixedOrderComparator<Integer> comparator2 = new
FixedOrderComparator<>(2, 3, 4);
+ assertFalse(comparator1.equals(comparator2));
+ }
+
+ @Test
+ void
expectFalseWhenBothComparatorsWithDifferentUnknownObjectBehavior() {
+ FixedOrderComparator<Integer> comparator1 = new
FixedOrderComparator<>();
+
comparator1.setUnknownObjectBehavior(FixedOrderComparator.UnknownObjectBehavior.BEFORE);
+ FixedOrderComparator<Integer> comparator2 = new
FixedOrderComparator<>();
+
comparator2.setUnknownObjectBehavior(FixedOrderComparator.UnknownObjectBehavior.AFTER);
+ assertFalse(comparator1.equals(comparator2));
+ }
+
+ @Test
+ void expectFalseWhenFixedOrderComparatorIsComparedWithNull() {
+ FixedOrderComparator<Integer> comparator = new
FixedOrderComparator<>();
+ assertFalse(comparator.equals(null));
+ }
+
+
+ @Test
+ void expectFalseWhenFixedOrderComparatorIsComparedWithOtherObject() {
+ FixedOrderComparator<Integer> comparator = new
FixedOrderComparator<>();
+ assertFalse(comparator.equals(new Object()));
+ }
+
+ @Test
+ void expectFalseWhenOneComparatorIsLocked() {
+ FixedOrderComparator<Integer> comparator1 = new
FixedOrderComparator<>(1, 2, 3);
+ FixedOrderComparator<Integer> comparator2 = new
FixedOrderComparator<>(1, 2, 3);
+ comparator2.compare(1, 2);
+ assertFalse(comparator1.equals(comparator2));
+ }
+
+ @Test
+ void expectFalseWhenOneComparatorsWithDuplicateItems() {
+ FixedOrderComparator<Integer> comparator1 = new
FixedOrderComparator<>(1, 2, 3);
+ FixedOrderComparator<Integer> comparator2 = new
FixedOrderComparator<>(1, 2, 3, 3);
+ assertFalse(comparator1.equals(comparator2));
+ }
+
+ @Test
+ void expectTrueWhenBothComparatorsAreLocked() {
+ FixedOrderComparator<Integer> comparator1 = new
FixedOrderComparator<>(1, 2, 3);
+ FixedOrderComparator<Integer> comparator2 = new
FixedOrderComparator<>(1, 2, 3);
+ comparator1.compare(1, 2);
+ comparator2.compare(1, 2);
+ assertTrue(comparator1.equals(comparator2));
+ }
+
+ @Test
+ void expectTrueWhenBothComparatorsWithoutAnyItems() {
+ FixedOrderComparator<Integer> comparator1 = new
FixedOrderComparator<>();
+ FixedOrderComparator<Integer> comparator2 = new
FixedOrderComparator<>();
+ assertTrue(comparator1.equals(comparator2));
+ }
+
+ @Test
+ void expectTrueWhenBothObjectsAreSame() {
+ FixedOrderComparator<Integer> comparator = new
FixedOrderComparator<>();
+ assertTrue(comparator.equals(comparator));
+ }
+ }
+
+ //
+ // Initialization and busywork
+ //
+
/**
* Top cities of the world, by population including metro areas.
*/
@@ -52,17 +127,13 @@ public class FixedOrderComparatorTest extends
AbstractComparatorTest<String> {
};
//
- // Initialization and busywork
+ // Set up and tear down
//
public FixedOrderComparatorTest() {
super(FixedOrderComparatorTest.class.getSimpleName());
}
- //
- // Set up and tear down
- //
-
/** Shuffles the keys and asserts that the comparator sorts them back to
* their original order.
*/
@@ -107,11 +178,6 @@ public class FixedOrderComparatorTest extends
AbstractComparatorTest<String> {
return Arrays.asList(topCities);
}
- @Override
- public String getCompatibilityVersion() {
- return "4";
- }
-
// public void testCreate() throws Exception {
// writeExternalFormToDisk((java.io.Serializable) makeObject(),
"src/test/resources/data/test/FixedOrderComparator.version4.obj");
// }
@@ -120,6 +186,11 @@ public class FixedOrderComparatorTest extends
AbstractComparatorTest<String> {
// The tests
//
+ @Override
+ public String getCompatibilityVersion() {
+ return "4";
+ }
+
@Override
public Comparator<String> makeObject() {
return new FixedOrderComparator<>(topCities);
@@ -178,6 +249,10 @@ public class FixedOrderComparatorTest extends
AbstractComparatorTest<String> {
assertComparatorYieldsOrder(keys, comparator);
}
+ //
+ // Helper methods
+ //
+
/**
* Tests whether or not updates are disabled after a comparison is made.
*/
@@ -195,10 +270,6 @@ public class FixedOrderComparatorTest extends
AbstractComparatorTest<String> {
"Should have thrown an UnsupportedOperationException");
}
- //
- // Helper methods
- //
-
@Test
public void testUnknownObjectBehavior() {
FixedOrderComparator<String> comparator = new
FixedOrderComparator<>(topCities);
@@ -233,5 +304,4 @@ public class FixedOrderComparatorTest extends
AbstractComparatorTest<String> {
assertEquals(-1, comparator.compare("New York", "Minneapolis"));
assertEquals( 0, comparator.compare("Minneapolis", "St Paul"));
}
-
}
