ArrayList contains method not work as I would expect?

Question

ArrayList use equals() in its contains method to see if provide object equals any item in the list as the document say:

Returns true if this list contains the specified element. More formally, returns true if and only if this list contains at least one element e such that (o==null ? e==null : o.equals(e)). see this

I have this class

class Foo
{
  private String value;
  public Foo(String value)
  {
    this.value = value;
  }

  @Override
  public boolean equals(Object o)
  {
    return o == null ? this.value == null : o.toString().equals(this.value);
  }
}

I want to use contains method to check if item exists like this

List<Foo> list = new ArrayList<Foo>();

Foo item1 = new Foo("item1");
Foo item2 = new Foo("item2");
list.add(item1);
list.add(item2);

System.out.println(item1.equals("item1"));       //return true
System.out.println(list.contains("item1"));      //false !! why?!

but contains method return false , while item1.equals("item1") return true.

why contains return false when it use equals method for provided object

`o` is the parameter passed into `contains` (in this case a `String`), `e` is the element of the `List` (your class `Foo`) - `o.equals(e)` is not the same as `e.equals(o)` — UnholySheep, Aug 10 '17 at 14:45
Find the actual object type in the list, not a string. I.E find a FOO in a list of FOOs and not a STRING in a list of FOOs — jaletechs, Aug 10 '17 at 14:47
Why would you use "@override" on your equals() method? your class doesn't extend any other class, I think that your equal() is lik any other method you would declare. — ziMtyth, Aug 10 '17 at 14:47
@ZiMtyth No. He's overriding it from the `Object` class. It is strongly recommended that you add this annotation, to be sure you have the good signature — Turtle, Aug 10 '17 at 14:49
@Ali FYI, the `Lists` implementations use the `hashCode` method to do a pre-sorting. You should definitely take a look at this link: https://stackoverflow.com/questions/8180430/how-to-override-equals-method-in-java **EDIT** Actually I tagged it as a dupe, because that's the reference for incorrect equals implementations. — Turtle, Aug 10 '17 at 14:54
I suggest you to read, why should we always override `hashCode` when we are overriding `equals`. I dont think it will work even after passing `new Foo("item1")`. https://www.quora.com/How-do-I-override-the-hashCode-and-equals-method-in-Java-HashMap/answer/Vivek-Singh-619?srid=Crub — viveksinghggits, Aug 10 '17 at 14:57

davidxxx · Accepted Answer · 2017-08-10T14:56:52.413

11

Your equals() implementation violates the symmetric principle :

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.

item1.equals("item1") returns true

while

"item1".equals(item1) returns false.

So you should not expect that Collection method such as contains() works in a consistent way.

As a general rule, equals() overriding should not try to interoperate with other classes but only with instances of the underlying class.

In your case, it works only for String parameter passed to the method.
You should rather make it working for Foo instances parameter:

  @Override
  public boolean equals(Object o) {
     if (!(o instanceof Foo){ 
       return false;
     }
     Foo other = (Foo) o;
     return Objects.equals(other.value, this.value);
  }

edited Aug 10 '17 at 14:56

answered Aug 10 '17 at 14:46

davidxxx

125,838
23
214
215

thanks I did got it now – Ali Faris Aug 10 '17 at 14:54
You are welcome. I edited with an alternative implementation. – davidxxx Aug 10 '17 at 14:58
1

This is the key answer. The root problem is the OP's implementation of `equal`. Don't feel bad though. It's a common error in Java – Dancrumb Aug 10 '17 at 15:42

Michael · Answer 2 · 2017-08-10T15:05:29.973

6

Your problem is that your equality is not symmetric. Foo == String does not imply String == Foo.

If you look at the implementation of ArrayList.contains you'll see that it calls objectToFind.equals(objectInList), which may be contrary to what you were expecting:

o.equals(elementData[i])

So in your case that's String.equals(Foo). Because String.equals will return false for anything that's not a String, ArrayList.contains returns false.

edited Aug 10 '17 at 15:05

answered Aug 10 '17 at 14:46

Michael

41,989
11
82
128

1

thanks this was helpful – Ali Faris Aug 10 '17 at 15:05

sapensadler · Answer 3 · 2017-08-10T14:58:29.393

5

Looking at the docs here the contains will use the equals method on the string "item1" and not the equals method on item1. e.g.

"item1".equals(item1)

and not

item1.equals("item1")

You could use list.contains(new Foo("item1")) instead.

edited Aug 10 '17 at 14:58

answered Aug 10 '17 at 14:45

sapensadler

386
5
11

thanks for your help – Ali Faris Aug 10 '17 at 14:55

score 4 · Answer 4 · answered Aug 10 '17 at 14:47

While item1.equals("item1") is true, "item1".equals(item1) will be false. Having a non-symmetric equals relation causes a lot of confusion.

In general, you want equals to only be true among classes you control (usually only the exact class you're comparing), so that you can ensure that the relation is symmetric. (And to avoid further confusion, always define hashCode whenever you define equals.)

ΦXocę 웃 Пepeúpa ツ · Answer 5 · 2017-08-10T14:58:26.830

2

YOUR Equals method is not considering the instance of the Object, so is wrongly implemented

you have a list of Foos and you want to know if the list contains a STRING

this:

System.out.println(list.contains("item1"));

is not the same as

System.out.println(list.contains(new Foo("item1")));

because

 new Foo("item1") never ever will return true on equals to the string "item1"

edit:

contains in the ArrayList is implemented as

    @Override
    public int indexOf(Object o) {
        E[] a = this.a;
        if (o == null) {
            for (int i = 0; i < a.length; i++)
                if (a[i] == null)
                    return i;
        } else {
            for (int i = 0; i < a.length; i++)
                if (o.equals(a[i]))
                    return i;
        }
        return -1;
    }

so this part is the important one

for (int i = 0; i < a.length; i++)
         if (o.equals(a[i]))
                  return i;

as you can see

  o.equals(a[i])

means

  String.equals(Foo)

which is NOT the same as what you did:

  Foo.equals(String)

you are breaking a rule of symmetry, that is the reason why is not working!

edited Aug 10 '17 at 14:58

answered Aug 10 '17 at 14:44

ΦXocę 웃 Пepeúpa ツ

47,427
17
69
97

i expect to return true since I override equals method in `Foo` – Ali Faris Aug 10 '17 at 14:45
*new Foo("item1") never ever will return true on equals to the string "item1"* that's just not true. You can cook up an equals() implementation to do exactly that – Tim Aug 10 '17 at 14:46
@ΦX __`new Foo("item1")` never ever will return true on equals to the string "item1"__ Why not? If I override my `equals` method in `Foo` class and make it return true always? You probably meant the other way around: __the string "item1" never ever will return true on equals to new Foo("item1")__ – Turtle Aug 10 '17 at 15:02
thanks for your clarification – Ali Faris Aug 10 '17 at 15:07

score 2 · Answer 6 · answered Aug 10 '17 at 14:46

2

Your equals implementation is clearly incorrect.

You should read chapter 3 of Joshua Bloch's "Effective Java" to learn how to override equals and hashcode correctly.

This will work better:

/**
 * Demo equals override
 * User: mduffy
 * Date: 8/10/2017
 * Time: 10:45 AM
 * @link https://stackoverflow.com/questions/45616794/arraylist-contains-method-not-work-as-i-would-except
 */
public class Foo {

    private final String value;

    public Foo(String value) {
        this.value = value;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof Foo)) return false;

        Foo foo = (Foo) o;

        return value != null ? value.equals(foo.value) : foo.value == null;
    }

    @Override
    public int hashCode() {
        return value != null ? value.hashCode() : 0;
    }

    @Override
    public String toString() {
        final StringBuilder sb = new StringBuilder("Foo{");
        sb.append("value='").append(value).append('\'');
        sb.append('}');
        return sb.toString();
    }
}

answered Aug 10 '17 at 14:46

duffymo

305,152
44
369
561

thanks for your help , I did tried it , but now it return `false` for both . downvote not from me – Ali Faris Aug 10 '17 at 14:52
@Michael No. for example, the TO's `equals` method isn't reflexive (not sure if that's the term): `a.equals(b)` is not always the same as `b.equals(a);`. This default is one of the reason that it doesn't work – Turtle Aug 10 '17 at 14:52
@Nathan Symmetric. There's a list of terms [on the javadoc](https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#equals(java.lang.Object)). Reflexive would be `x.equals(x) == true` – Michael Aug 10 '17 at 14:54
Very absolute a term. Joshua Bloch lays out exactly how to do it properly. If you don't follow his recommendation, it's wrong. Simple. His implementation can be proven to be reflexive, symmetric, and transitive. All essential for getting it right. – duffymo Aug 10 '17 at 14:54
@Michael Thanks. On the side, the equals method **must** also be reflexive :p At least it's expected to be. An implementation of `equals` that break any of those criterias is by definition **wrong**, so I think @duffymo 's wording was OK. – Turtle Aug 10 '17 at 14:56

Juan Carlos Mendoza · Answer 7 · 2017-08-10T14:51:16.800

1

the problem is in your equals override: you are comparing o.toString() vs this.value.

to get it working this are your options:

Override the toString method on your Foo class to return value
Change the last part to this.value.equals(o.getValue()) // have to create the getter

edited Aug 10 '17 at 14:51

answered Aug 10 '17 at 14:44

Juan Carlos Mendoza

5,736
7
25
50

ArrayList contains method not work as I would expect?

7 Answers7