hashset return false when should return true

Question

I'm programming a Maze and I have some problems.

I have:

HashSet<State> closedList = HashSet<State>(); //it hold State objects

My State class look like this:

public class State implements Comparable<State>{
private double f;
private double g;
private String state; 
private State prev;
.
.
.

closedList.add(state);
closedList().contains(state); // this equals true

but when I do this:

State temp = State(0,0,"");
temp.setStateName(state.getStateName());

closedList().contains(temp); // this equals false

I have implemented equals and hashCode in State:

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

@Override
public boolean equals(Object object){
    if(this.state == object){
        return true;
    }
    if(object == null || object.getClass() != this.getClass()){
        return false;
    }
    return false;
}

And do you expect anything different? Have a good, hard look at your code again... (hint: `.equals()`) — fge, Apr 09 '14 at 15:27
my apologize, im very noob at java, what the problem in equals? — bitWise, Apr 09 '14 at 15:31
Your `equals` method makes no sense unless `this.state == this == object`. — chrylis -cautiouslyoptimistic-, Apr 09 '14 at 15:40
Simple enough: you expect two new objects to be initialized with the same state to point to _the same reference_ (`==`); but this is not the case. And this is what `.equals()` is for — fge, Apr 09 '14 at 15:41
In my opinion, the best way to learn is by reading the open source code. Guava javadocs : http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Objects.html#equal(java.lang.Object, java.lang.Object) — Anupam Saini, Apr 09 '14 at 15:45
You are comparing a `String` to a `State` reference. It will never be true. — anonymous, Apr 09 '14 at 15:49
The equals method is the place for you to 'judge' whether the one object and another object as 'essentially' equal. As another hint, if we have a class called Person, and we judge whether two Persons are essentially the same based on the person's name, then the `equals` method should look like this: public boolean equals(Object other) { Person otherPerson = (Person) other; if (this.name.equals(otherPerson.name)) { return true; } else { return false; } } — suhe_arie, Apr 09 '14 at 15:55

score 1 · Accepted Answer · edited May 23 '17 at 12:13

closedList().contains(state); // this equals true

This is a red herring, it only returns true because HashSet checks with == before it makes a call to equals.

What you should try is something like this:

State temp = new State(0, 0, "");
System.out.println(temp.equals(temp));

And you will find this returns false. Why is that? Well let's follow the logic through.

First, you have this check:

if(this.state == object){
    return true;
}

If you really intended this to be the way it is, it means you were expecting equals to be called with the String state as the argument, like this:

temp.equals(temp.getStateName())

(And it's the case the above call would return true.) This is incorrect, one would not expect equals to return true for unrelated classes (and in terms of the equals contract, it's the case this is not symmetric). I assume this is unintended and just like a mistake. You should think more carefully about what your code is doing when you are writing it.

Also you should be comparing Strings with equals, not ==.

Then there is this construct:

if(object == null || object.getClass() != this.getClass()){
    return false;
}
return false;

This is pointless because first what it implies logically is this, returning false either way:

if(object == null || object.getClass() != this.getClass()){
    return false;
} else {
    return false;
}

And, second, combined with the earlier check it is not particularly logical:

if(this.state == object)
    return true;
if(object.getClass() != this.getClass())
    return false;

This is returning true if object is == to a String but returning false if object's class is not State. These are mutually exclusive.

So the equals implementation you wrote doesn't work. The correct equals to match your hashCode is like this:

@Override
public boolean equals(Object object){
    if(object == null || object.getClass() != this.getClass()){
        return false;
    }

    State other = (State)object;
    return this.state.equals(other.state);
}

First check that the object is not null and that its class is State (you had that part right), then check that the state member is equal to the other object's state member.

hashset return false when should return true

1 Answers1