List.contains() fails while .equals() works
Asked Answered
T

4

39

I have an ArrayList of Test objects, which use a string as the equivalency check. I want to be able to use List.contains() to check whether or not the list contains an object that uses a certain string.

Simply:

Test a = new Test("a");
a.equals("a"); // True

List<Test> test = new ArrayList<Test>();
test.add(a);
test.contains("a"); // False!

Equals and Hash function:

@Override
public boolean equals(Object o) {
    if (o == null) return false;
    if (o == this) return true;
    if (!(o instanceof Test)) {
        return (o instanceof String) && (name.equals(o));
    }
    Test t = (Test)o;
    return name.equals(t.GetName());
}

@Override
public int hashCode() {
    return name.hashCode();
}

I read that to make sure contains works for a custom class, it needs to override equals. Thus it's super strange to me that while equals returns true, contains returns false.

How can I make this work?

Full code

Tridimensional answered 16/2, 2016 at 6:47 Comment(5)
a class should only test equals with the same class.Pastis
Your idea of what equals is suppose to do is wrong, you should be comparing Test with another instance of TestDwarf
only the reverse would work, because "a".equals(new Test("a")) == falsePopularly
also, the fact that "a".equals(new Test("a")) != new Test("a").equals("a") is a problem.Popularly
@bayou.io The contract for Object.equals tacitly requires it, due to the commutative property of non-null equality.Centerboard
T
41

Just because your Test's equals may return true when you pass a String to it doesn't mean that String's equals will ever return true when you pass a Test instance to it. In fact, String's equals can only return true when the instance passed to it is another String :

public boolean equals(Object anObject) {
    if (this == anObject) {
        return true;
    }
    if (anObject instanceof String) { // the passed instance must be a String
        String anotherString = (String)anObject;
        int n = value.length;
        if (n == anotherString.value.length) {
            char v1[] = value;
            char v2[] = anotherString.value;
            int i = 0;
            while (n-- != 0) {
                if (v1[i] != v2[i])
                    return false;
                i++;
            }
            return true;
        }
    }
    return false;
}

ArrayList's contains calls indexOf which uses the equals method of the searched instance (the String "a" in your example), not the element type of the List (which is Test in your case) :

public int indexOf(Object o) {
    if (o == null) {
        for (int i = 0; i < size; i++)
            if (elementData[i]==null)
                return i;
    } else {
        for (int i = 0; i < size; i++)
            if (o.equals(elementData[i])) // o in your case is a String while
                                          // elementData[i] is a Test
                                          // so String's equals returns false
                return i;
    }
    return -1;
}
Tomasatomasina answered 16/2, 2016 at 6:49 Comment(4)
new Test("a").equals("a") returns true, while "a".equals(new Test("a")) returns false, nice explaing! thanks Eran!Turbit
Ahh, that makes a lot of sense now. So essentially the only way to solve this (and the only right way to go about this) is to write test.contains(new Test("a"))?Tridimensional
I assume the reason we have o.equals(elementData[i] instead of its inverse is because we have a guarantee that o != null and we have no such guarantee with the object[]?Thy
@BenKnoble o is guaranteed not to be null due to the specific implementation of indexOf. An alternative implementation that would also work is to have a single loop that checks if each elementData[i] is null or not, and run elementData[i],equals(o) if it's not null. That implementation would require more null checks when o != null, which might be the reason it wasn't chosen.Tomasatomasina
E
27

equals() should always be commutative, i.e. a.equals(b) and b.equals(a) should always return the same value. Or symmetric, as the javadoc of equals() calls it:

The equals method implements an equivalence relation on non-null object references:

  • It is reflexive: for any non-null reference value x, x.equals(x) should return true.
  • It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
  • It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
  • It is consistent: for any non-null reference values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
  • For any non-null reference value x, x.equals(null) should return false.

Unfortunately, even the Java Runtime Library gets this one wrong. Date.equals(Timestamp) will compare the millisecond values, ignoring the nanoseconds present in the Timestamp, while Timestamp.equals(Date) returns false.

Extension answered 16/2, 2016 at 7:3 Comment(2)
+1 Keeping the asymmetric equals would make the code unnecessarily fragile and hard to maintain because of dependency on which way round tests are done.Kramatorsk
There is even bigger assymetry in JDK - Map interface. If you have ever wondered why Map has get(Object key) instead of get(K key) - exactly to allow passing random classes which happen to have broken equality working against K instances. In one version of java Sun has changed this order (so value inside map was compared to passed key instead of other way around) and it has broken so many applications they had to fall back on that.Bristle
C
19

The problem is that List<E>.contains(object o) is documented to return true:

if and only if this list contains at least one element e such that (o==null ? e==null : o.equals(e)).

(From https://docs.oracle.com/javase/8/docs/api/java/util/List.html#contains-java.lang.Object-)

Note that it doesn't perform the test as e.equals(o) which is what would be necessary for your test to work. Your equals method fails to work commutatively ('symmetrically' using the terms from the Java docs).

Java documents that the equals() method for a class must follow these rules:

The equals method implements an equivalence relation on non-null object references:

  • It is reflexive: for any non-null reference value x, x.equals(x) should return true.
  • It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
  • It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
  • It is consistent: for any non-null reference values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
  • For any non-null reference value x, x.equals(null) should return false.
Cleaning answered 16/2, 2016 at 6:55 Comment(0)
J
5

If you write

test.contains(new Test("a")); 

then it will surely return true. You are checking for string object in list of Test.

Jill answered 16/2, 2016 at 7:7 Comment(1)
So we need to create a new object with using contains call?Redfield

© 2022 - 2024 — McMap. All rights reserved.