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;
}

Answer 1

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.