views:

9807

answers:

4

I ran into an interesting (and very frustrating) issue with the equals() method today which caused what I thought to be a well tested class to crash and cause a bug that took me a very long time to track down.

Just for completeness, I wasn't using an IDE or debugger - just good old fashioned text editor and System.out's. Time was very limited and it was a school project.

Anyhow -

I was developing a basic shopping cart which could contain an ArrayList of Book objects. In order to implement the addBook(), removeBook(), and hasBook() methods of the Cart, I wanted to check if the Book already existed in the cart. So off I go -

public boolean equals(Book b) {
    ... // More code here - null checks
    if (b.getID() == this.getID()) return true;
    else return false;
}

All works fine in testing. I create 6 objects and fill them with data. Do many adds, removes, has() operations on the Cart and everything works fine. I read that you can either have equals(TYPE var) or equals(Object o) { (CAST) var } but assumed that since it was working, it didn't matter too much.

Then I ran into a problem - I needed to create a Book object with only the ID in it from within the Book class. No other data would be entered into it. Basically the following:

public boolean hasBook(int i) {
    Book b = new Book(i);
    return hasBook(b);
}

public boolean hasBook(Book b) {
    // .. more code here
    return this.equals(b);
}

All of a sudden, the equals(Book b) method no longer works. This took a VERY long time to track down without a good debugger and assuming the Cart class was properly tested and correct. After swaapping the equals() method to the following:

public boolean equals(Object o) {
    Book b = (Book) o;
    ... // The rest goes here   
}

Everything began to work again. Is there a reason the method decided not to take the Book parameter even though it clearly was a Book object? The only difference seemed to be it was instantiated from within the same class, and only filled with one data member. I'm very very confused. Please, shed some light?

+34  A: 

In Java, the equals() method that is inherited from Object is:

public boolean equals(Object other);

In other words, the parameter must be of type Object.

The ArrayList uses the correct equals method, where you were always calling the one that didn't properly override Object's equals.

Not overriding the method correctly can cause problems.

I override equals the following everytime:

@Override
public boolean equals(Object other){
    if (other == null) return false;
    if (other == this) return true;
    if (this.getClass() != other.getClass())return false;
    MyClass otherMyClass = (MyClass)other;
    ...test other properties here...
}

The use of the @Override annotation can help a ton with silly mistakes.

Use it whenever you think you are overriding an super class's or interface's method. That way, if you do it wrong you will get a compile error.

jjnguy
This is a good argument in favour of the @Override annotation... if the OP had use @Override his compiler would have told him that he wasn't actually overriding a parent class method...
Cowan
Totally agreed!!!!! I will add that to my answer in a sec
jjnguy
Was never aware of the @Override, thanks for that! I'd also like to add that overriding hashCode() really should have been done and may have spotted the error sooner.
Josh Smeaton
Some IDEs (e.g. Eclipse) can even autogenerate equals() and hashcode() methods for you based on the class member variables.
sk
Eclipse usually does a good job too.
jjnguy
+1 for @Override. And--that last edit made me laugh. :)
Michael Myers
thanks, it came a month late. I'm surprised nobody noticed it until now.
jjnguy
Maybe they thought it was intentional.
Michael Myers
A: 

I am sorry I am kind of sleepy .. but you can read here to get an idea of your problem :) http://forums.sun.com/thread.jspa?threadID=620716&messageID=3497835

Wael Awada
+14  A: 

jjnguy has the right idea. I'd also like to add that you should always override hashCode whenever overriding equals.

Julie
I wonder why you got a downvote. Its not an answer to the question...but it can't be unhelpful. Its always good to have a reminder.
jjnguy
thanks for your support :-)
Julie
+1  A: 

Slightly off-topic to your question, but it's probably worth mentioning anyway:

Commons Lang has got some excellent methods you can use in overriding equals and hashcode. Check out EqualsBuilder.reflectionEquals(...) and HashCodeBuilder.reflectionHashCode(...). Saved me plenty of headache in the past - although of course if you just want to do "equals" on ID it may not fit your circumstances.

I also agree that you should use the @Override annotation whenever you're overriding equals (or any other method).

Phill Sacre
If you're an eclipse user, you can also go `right click -> source -> generate hashCode() and equals()`,
tunaranch