154

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.books.contains(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?

Rob Audenaerde
  • 19,195
  • 10
  • 76
  • 121
Josh Smeaton
  • 47,939
  • 24
  • 129
  • 164
  • 1
    I am aware that I violated the 'Contract' regarding overriding the equals methods by being reflective - however I needed a quick way to check if the object existed in the ArrayList without using generics. – Josh Smeaton Oct 09 '08 at 04:29
  • 1
    This is a good lesson to learn about Java and equals – jjnguy Oct 09 '08 at 04:34

8 Answers8

335

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. This is called overriding; your method public boolean equals(Book other) does what is called overloading to the equals() method.

The ArrayList uses overridden equals() methods to compare contents (e.g. for its contains() and equals() methods), not overloaded ones. In most of your code, calling the one that didn't properly override Object's equals was fine, but not compatible with ArrayList.

So, 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 (!(other instanceof MyClass)) 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 a super class' or interface's method. That way, if you do it the wrong way, you will get a compile error.

Mew
  • 368
  • 1
  • 11
jjnguy
  • 136,852
  • 53
  • 295
  • 323
  • 33
    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 Oct 09 '08 at 04:36
  • 1
    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 Oct 09 '08 at 04:52
  • 5
    Some IDEs (e.g. Eclipse) can even autogenerate equals() and hashcode() methods for you based on the class member variables. – sk. Oct 09 '08 at 05:48
  • 1
    `if (!(other instanceof MyClass))return false;` returns `false` if `MyClass` extends the other class. But it wouldn't return `false` if the other class extended `MyClass`. Shouldn't `equal` be less contradictory? – Robert Jul 14 '12 at 01:16
  • 20
    When using instanceof the previous nullcheck is redundant. – Mateusz Dymczyk Feb 02 '13 at 03:28
  • 1
    From the question **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.** The points about IDEs helping with @Override highlight that using an IDE and debugger are probably even more important when time is very limited. – Joshua Taylor Jul 18 '14 at 18:45
  • 1
    @Robert 's comment and @Nikel8000 's answer raise a VERY important point. Unless your class is final, you're breaking the contract on `equals()` that `o1.equals(o2)` should return true only if `o2.equals(o1)` also returns true. if `o1` is an instance of a subclass of `o2`, `o1 instanceof o2` returns true (and so does `o1.equals(o2)`), but `o2 instanceof o1` returns false, `o2.equals(o1)` returns false, and the contract is broken. – Blueriver Jul 07 '15 at 23:39
  • 1
    @Blueriver `o2 instanceof o1` isn't even legal syntax. The only potential issue would be if a subclass implemented `equals()` more restrictively. – shmosel Aug 14 '17 at 19:56
110

If you use eclipse just go to the top menu

Source --> Generate equals() and hashCode()

Fred
  • 1,486
  • 1
  • 9
  • 6
11

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

  • 4
    If you're an eclipse user, you can also go `right click -> source -> generate hashCode() and equals()`, – tunaranch Nov 06 '08 at 04:31
  • 1
    Am I right that these method is executed at runtime? Won't we have performance issues in case if we traversing a big collection with items checking them for equality to some other item because of reflection? – Gaket Jan 03 '18 at 05:15
5

Another fast solution that saves boilerplate code is Lombok EqualsAndHashCode annotation. It is easy, elegant and customizable. And does not depends on the IDE. For example;

import lombok.EqualsAndHashCode;

@EqualsAndHashCode(of={"errorNumber","messageCode"}) // Will only use this fields to generate equals.
public class ErrorMessage{

    private long        errorNumber;
    private int         numberOfParameters;
    private Level       loggingLevel;
    private String      messageCode;

See the options avaliable to customize which fields to use in the equals. Lombok is avalaible in maven. Just add it with provided scope:

<dependency>
    <groupId>org.projectlombok</groupId>
    <artifactId>lombok</artifactId>
    <version>1.14.8</version>
    <scope>provided</scope>
</dependency>
borjab
  • 11,149
  • 6
  • 71
  • 98
1

in Android Studio is alt + insert ---> equals and hashCode

Example:

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

    Proveedor proveedor = (Proveedor) o;

    return getId() == proveedor.getId();

}

@Override
public int hashCode() {
    return getId();
}
David Hackro
  • 3,652
  • 6
  • 41
  • 61
1

Consider:

Object obj = new Book();
obj.equals("hi");
// Oh noes! What happens now? Can't call it with a String that isn't a Book...
bcsb1001
  • 2,834
  • 3
  • 24
  • 35
  • 1
    @Elazar How so? `obj` is declared as an `Object`. The point of inheritance is that you can then assign a `Book` to `obj`. After that, unless you suggest that an `Object` should not be comparable to a `String` via `equals()`, this code should be perfectly legal and return `false`. – bcsb1001 Jun 29 '16 at 14:59
  • I suggest exactly that. I believe it is pretty widely accepted. – Elazar Jun 30 '16 at 10:12
0

the instanceOf statement is often used in implementation of equals.

This is a popular pitfall !

The problem is that using instanceOf violates the rule of symmetry:

(object1.equals(object2) == true) if and only if (object2.equals(object1))

if the first equals is true, and object2 is an instance of a subclass of the class where obj1 belongs to, then the second equals will return false!

if the regarded class where ob1 belongs to is declared as final, then this problem can not arise, but in general, you should test as follows:

this.getClass() != otherObject.getClass(); if not, return false, otherwise test the fields to compare for equality!

Luboš Turek
  • 6,273
  • 9
  • 40
  • 50
Nikel8000
  • 33
  • 1
  • 3
    See Bloch, *Effective Java,* Item 8, a large section discussing issues with overriding the `equals()` method. He recommends against using `getClass()`. The main reason is that doing so breaks the Liskov Substitution Principle for subclasses that don't affect equality. – Stuart Marks Jul 29 '15 at 15:53
-1

recordId is property of the object

@Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Nai_record other = (Nai_record) obj;
        if (recordId == null) {
            if (other.recordId != null)
                return false;
        } else if (!recordId.equals(other.recordId))
            return false;
        return true;
    }
vootla561
  • 11
  • 1