ArrayList's contains() method always returns false with custom object

Question

I'm having some troubles with getting on with my code, I'll give you an simple example (though it's going to be a little more complex, this simple code doesn't work properly either).

class Sign {

  private String char;
  private Integer freq;

  public Sign(String c) {
  this.char = c; 
  }

  @Override
  public boolean equals(Object o) {

   String check = (String)o;
   return check.equals(this.char);
  }

  @Override
  public int hashCode() {

    int hash = 7;
    hash = 31 * hash + this.char.hashCode();
    return hash;
}

}

I assume that there's always will be a String in equals method for simplicity reasons. There's some hashCode() also to make sure that the contains() method will work and here's the test itself:

    ArrayList<Sign> queueOfSigns = new ArrayList<>();

    Sign test = new Sign("C");
    String c = "C";
    queueOfSigns.add(test);

    if(queueOfSigns.contains("C"))
        System.out.println("I am here!");

No matter what, this simple test-code always returns false in that case - so "I'm here" message never appears. I've been trying some different ways of approach my code but it was because the idea of this is to get single characters from String text and check whether the single character is already present in the ArrayList. Nevertheless - without getting this simple test working properly I can't move on, so I would like to ask you - what am I missing. It's my first time actually with using equals() and hashCode() methods to get my own object working properly with contains() method.

Most likely the issue is that new Sign("C").equals("C") does not return the same thing as "C".equals(new Sign("C")). To work properly, equals needs to be symmetric: a.equals(b) == b.equals(a) should always be true. — Daniel Pryden
– Daniel Pryden, Commented Sep 13, 2018 at 16:06

T.J. Crowder · Accepted Answer · 2018-09-13 16:07:33Z

5

Your equals implementation is incorrect. equals has a specific contract; that code attempts to violate that contract. From the documentation:

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.

There's no way to make an instance of your Sign class equals to a string.

answered Sep 13, 2018 at 16:07

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

10 Comments

Daniel Pryden Over a year ago

It's certainly possible to make an object report that it is equal to an object of another type. It just won't work correctly if the other object doesn't also make itself equal to your object. There are a few places where you can bend the rules (e.g. some projects have used dedicated "lookup objects" that are equal "enough", for exactly cases like this), but bending the rules is a very tricky proposition.

T.J. Crowder Over a year ago

@DanielPryden - Right, and a useful observation. But String's equals doesn't and isn't going to support the OP's Sign class, so... :-)

John Bollinger Over a year ago

For completeness, among those properties, the OP's equals() implementation exhibits only transitivity. And where it's broken, it's really broken: it has a nasty habit of throwing ClassCastExceptions and NullPointerExceptions.

T.J. Crowder Over a year ago

@LowProfile - I wouldn't use an ArrayList at all. I'd use a Map and store the Sign object using its char property as the key. Then you use containsKey to find out if the map has the object, or get to get the object based on the key.

LowProfile Over a year ago

@T.J. Crowder - in the end I might use map - that was my first idea. I just wanted to have some tryout with arraylist and own equals(), hashCode().

|

Sweeper · Accepted Answer · 2018-09-13 16:37:08Z

1

Your equals method is implemented incorrectly. It violates the general contract of Object.equals:

It is not reflexive - since it throws an exception when the argument is not a string, x.equals(x) where x is a Sign will crash with an exception.
It is not symmetric - x.equals(y) does not return the same value as y.equals(x), if y is a string and x is a Sign
It is not consistent - since it can possibly throw exceptions when the argument is not a string, not just return true or false.

On a low level of abstraction, the cause of this problem is the implementation of contains. As per the docs:

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)).

ArrayList actually calls o.equals(e) with o being the string you passed in. So it actually calls the equals method in String.

If contains called e.equals(o), then your program would have printed "I'm here", but your equals still violates the contract.

A better equals implementation is something like this:

@Override
public boolean equals(Object o) {
    if (o == null) {
        return false;
    }

    if (o.getClass() == this.getClass()) {
        Sign other = (Sign)o;
        return other.$char.equals($char); // I have renamed 'char' to '$char' since the former is not a valid identifier
    } else {
        return false;
    }
}

And your client code:

    ArrayList<Sign> queueOfSigns = new ArrayList<>();

    Sign test = new Sign("C");
    Sign c = new Sign("C");
    queueOfSigns.add(test);

    if(queueOfSigns.contains(c))
        System.out.println("I am here!");

EDIT:

I think this is what you are looking for:

arrayList.stream()
    .filter(x -> x.getChar().equals("C"))
    .findFirst().isPresent() // this returns true if a sign with C is found in the array list

edited Sep 13, 2018 at 16:37

answered Sep 13, 2018 at 16:18

Sweeper

292k23 gold badges260 silver badges438 bronze badges

3 Comments

T.J. Crowder Over a year ago

Even using instanceof in equals is dodgy. If I create a class NiftySign that extends Sign, a NiftySign instance isn't equal to a Sign instance, but Sign's equals (per the above) would think it was.

LowProfile Over a year ago

Thanks, I get the problem, but the code won't do what I'm actually looking for. Here's the thing - I need to pass single letter (for example) as a String or a Character to contains() method of ArrayList - which has to contain Sign objects (ArrayList<Sign>). The problem persist, cause I just wanted to have my own, overridden equals() method that would actually compare String from Sign object to given String, like in the example. I just wanted to avoid "instanceof" because in my cause there is no way of passing anything alse - even though it's not a good practice in any way - I know.

Sweeper Over a year ago

Then you shouldn't override equals. You need to find an element in the array list that has a char field of "C". See the edit. @LowProfile

Collectives™ on Stack Overflow

ArrayList's contains() method always returns false with custom object

2 Answers 2

10 Comments

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

10 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related