Should my equals/hashCode method examine more than the object IDs?

In my application, I have model classes of the following form:

class Book
{
    private int ID;
    private String title;

    //other code
}

Now my question is two part:

1. Is the following a good implementation of the equals() method?

   public boolean equals(Object o)
   {
       if(o == null)
       {
           return false;
       }
   
       if(!(o instanceof Book))
       {
           return false;
       }
   
       Book other = (Book)o;
   
       if(o.getID() == ID)
       {
           return true;
       }
       return false;
   }
   

   I know that equals() implementation largely depends on my application business logic. But If two Books have the same ID then they ideally must be the same Book. Hence I am confused as to whether I should check for equality for other value fields as well [title, price etc].

2. Is this a good implementation of the hashCode() method:

   public int hashCode()
   {
       return ID;
   }
   

   My thinking is that different books will have different IDs and if two books have the same ID they they are equal. Hence the above implementation will ensure a good distribution of the hashcode in context of my applicati开发者_如何学Con.

Just wanted to add some coment over the previous answers. The contract of equals mentions that it must be symmetric. This means that a.equals(b) iff b.equals(a).

That's the reason why instanceof is usually not used in equals (unless the class is final). Indeed, if some subclass of Book (ComicsBook for example) overrides equals to test that the other object is also an instance of ComicsBook, you'll be in a situation where a Book instance is equal to a ComicsBook instance, but a ComicsBook instance is not equal to a Book instance.

You should thus (except when the class is final or in some other rare cases) rather compare the classes of the two objects:

if (this.getClass() != o.getClass()) {
    return false;
}

BTW, that's what Eclipse does when it generates hashCode and equals methods.

If you use Hibernate then you have to consider some Hibernate related concerns.

Hibernate create Proxies for Lazy Loading.

• always use getter to access the properties of the other object (you already done that)
• even if it is correct to use if (!this.getClass().equals(o.getClass())) { return false;} in a normal Application it WILL fail for hibernate Proxy (and all other proxies). The reason is if one of the two is a proxy, there classes will never be equals. So the test if(!(o instanceof Book)){return false;} is what you need.

If you want to do it in the symmetric way than have a look at org.hibernate.proxy.HibernateProxyHelper.getClassWithoutInitializingProxy() with help of this class you can implement:

if (!HibernateProxyHelper.getClassWithoutInitializingProxy(this)
     .equals(HibernateProxyHelper.getClassWithoutInitializingProxy(o))) {
   return false;
}

• An other problem maybe is the ID -- when you assign the id not with the creation of an new object but later on while storing them, then you can run into trouble. Assume this scenario: You create a new Book with id 0, then put the Book in a HashSet (it will assigned to an bucket in the hashset depending on the hashcode), later on you store the Book in the Database and the id is set to lets say 1. But this changes the hashcode, and you will have problems to find the entity in the set again. -- If this is a problem or not for you, strongly depends on your application, architecture and how you use hibernate.

Don't do this:

if(o.getID() == ID)

ID is an Integer object, not a primitive. Comparing two different but equal Integer objects using == will return false.

Use this:

if(o.getID().equals(ID))

You'd also need to check for ID being null.

Other than that, your logic is fine - you're sticking with the contract that says two equal objects must have the same hashcode, and you've made the business logic decision as to what equality means -a decision that only you can make (there's no one correct answer).

It is no good idea to compare the two Integers like

if(o.getID() == ID) ...

This tests for identity. What you want is a test for equality:

if(ID!=null && ID.equals(o.getID())) ...

Just a note: Your equals method

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

    if(!(o instanceof Book))
    {
        return false;
    }

    Book other = (Book)o;

    if(other.getID() == ID)
    {
        return true;
    }
    return false;
}

can be (equivalently) shorter written like this:

public boolean equals(Object o) {
    return (o instanceof Book) &&
       ((Book)o).getID == ID;
}

Other than this, if your IDs are all different for different books (and same for same books), this is a good implementation.

(But note JB Nizet's remark: To make sure this stays symmetric, make equals (or the whole class) final.)

that depends !

i think this kind of implementation is ok, if you can handle the condition below,

you new two books, these two books have the same title, they are same book in fact, but you don't save them into database, so these two books don't have ids yet, the equals will fall when you compare them