35

I am having an issue where I make an ArrayList of Foo objects, I override the equals method, and I cannot get the contains method to call the equals method. I have tried overriding equals and hashcode together, but it still doesn't work. I'm sure there is a logical explanation to why this is, but I cannot figure it out at the moment on my own lol. I just want a way to see if the list contains the specified id.

Here's some code:

import java.util.ArrayList;
import java.util.List;

public class Foo {

    private String id;


    public static void main(String... args){
        Foo a = new Foo("ID1");
        Foo b = new Foo("ID2");
        Foo c = new Foo("ID3");
        List<Foo> fooList = new ArrayList<Foo>();
        fooList.add(a);
        fooList.add(b);
        fooList.add(c);
        System.out.println(fooList.contains("ID1"));
        System.out.println(fooList.contains("ID2"));
        System.out.println(fooList.contains("ID5"));
    }   

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

    @Override
    public boolean equals(Object o){
        if(o instanceof String){
            String toCompare = (String) o;
            return id.equals(toCompare);
        }
        return false;
    }



    @Override
    public int hashCode(){
        return 1;
    }
}

OUTPUT: false false false

Reid Mac
  • 2,411
  • 6
  • 37
  • 64
  • Why would you check foolish to contain a string? – Prabhat Gaur Feb 17 '21 at 09:56
  • 1
    I asked this about 9 years ago. I know now that it doesn't make sense, but at the time, I did not understand. Thanks to the answers to this question, I now understand how .equals() works. – Reid Mac Feb 17 '21 at 16:16

3 Answers3

48

This is because your equals() is not symmetric:

new Foo("ID1").equals("ID1");

but

"ID1".equals(new Foo("ID1"));

is not true. This violates the equals() contract:

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

  • [...]

  • 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 not reflexive either:

  • It is reflexive: for any non-null reference value x, x.equals(x) should return true.
Foo foo = new Foo("ID1");
foo.equals(foo)  //false!

@mbockus provides correct implementation of equals():

public boolean equals(Object o){
  if(o instanceof Foo){
    Foo toCompare = (Foo) o;
    return this.id.equals(toCompare.id);
  }
  return false;
}

but now you must pass instance of Foo to contains():

System.out.println(fooList.contains(new Foo("ID1")));
System.out.println(fooList.contains(new Foo("ID2")));
System.out.println(fooList.contains(new Foo("ID5")));

Finally you should implement hashCode() to provide consistent results (if two objects are equal, they must have equal hashCode()):

@Override
public int hashCode() {
    return id.hashCode();
}
Community
  • 1
  • 1
Tomasz Nurkiewicz
  • 334,321
  • 69
  • 703
  • 674
  • 1
    @ReidMac: I was wrong, it is about `equals()` not being symmetric, have a look at my edit. `hashCode()` has nothing to do in this case, but still you should follow this principle – Tomasz Nurkiewicz Feb 16 '12 at 21:05
  • Strange that we need to use this `new Foo("ID1");` convention to use custom equals method. Any reason behind this? – coretechie Mar 22 '17 at 09:36
11

Your equals method needs to be altered along with overriding the hashCode() function. Currently you're checking to see if the object you're comparing to is an instanceof String, when you need to be checking for Foo objects.

public boolean equals(Object o){
    if(o instanceof Foo){
        Foo toCompare = (Foo) o;
        return this.id.equals(toCompare.id);
    }
    return false;
}

If you're using Eclipse, I would recommend having Eclipse generate the hashCode and equals for you by going to Source -> Generate hashcode() and equals()...

Mike Bockus
  • 2,079
  • 17
  • 23
6

You should implement hashCode

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

even though the contains works for ArrayList without it. Your big problems are that your equals expects String, not Foo objects and that you ask for contains with Strings. If the implementation asked each eject in the list if they were equal to the string you send, then your code could work, but the implementation asks the string if it is equal to your Foo objets which it of course isn't.

Use equals

@Override
public boolean equals(Object o){
    if(o instanceof Foo){
        String toCompare = ((Foo) o).id;
        return id.equals(toCompare);
    }
    return false;
}

and then check contains

System.out.println(fooList.contains(new Foo("ID1")));
Roger Lindsjö
  • 11,330
  • 1
  • 42
  • 53
  • 2
    Using HashSets, my equals() method wasn't being called on contains() checks at all until I added the hashCode() method too. – frances Apr 06 '18 at 17:23