JBossDeveloper

As I stated earlier in this thread it is a bad idea to remove the lines

if (getClass() != obj.getClass())
    return false;

This check is there so that the next line of code will not throw a ClassCastException if the object you are comparing is not of the right type.

However, using the getClass() may be causing issues when dealing with proxies.

Instead write it like this.

if (! obj instanceof MyClass) {
    return false;
}

This will handle proxies and the like (see second highest voted response in the stackoverflow link below).

I would also suggest spending some time reading about implementing hashcode and equals.

At the moment your implementation breaks some of the fundamental rules about how they should work.

1. Hashcode changes when you persist the object.

2. Your equals assumes all unpersisted objects are equal.

Assuming your id is set at persist time (as that is the standard way hibernate works). Your implementation means that all entities of the same type without an id are equal.

This is not true. As when you persist each of those entities they will no longer be equal.

And the way your hashcode is implemented could cause you undesired behaviour if you persist a new obect that you have in a hash set.

For example.

MyEntity ent = new MyEntity();
ent.setField1('value');

Set<MyEntity> set = new HashSet<MyEnttiy>();
set.add(ent);

assert(set.contains(ent)); //So far so good

entityManager.persist(ent);

assert(set.contains(ent)); //As far as I know this will fail because the hashcode has changed since it was added to the set.

However, it gets much worse then that. (Assuming my logic is correct)

Because your equals method assumes that all null id instances are equal.

MyEntity ent = new MyEntity();
ent.setField1('value');

MyEntity ent2 = new MyEntity();
ent2.setField1('value2);

assert(ent.equals(ent2)); //Your implementation will return true

Set<MyEntity> set = new HashSet<MyEnttiy>();
set.add(ent);
set.add(ent2);

assert(set.size()==2); //FAIL. Both objects were equal so the set only contains 1 instance

Anyway, I could be wrong as I may be missing something. But, I would recommend taking a read of the following links.

http://stackoverflow.com/questions/27581/overriding-equals-and-hashcode-in-java

http://web.archive.org/web/20110622072109/http://java.sun.com/developer/Books/effectivejava/Chapter3.pdf

@Tim Evers :

You are right about those points. I was trying to explain why some people couldn't make it work, while overriding hashCode() and equals() methods. So I left my code a little bit sketchy. They are not the perfect overrides. In fact as you've said they are faulty..

The link you give (stackoverflow one), helped me a lot, so thanks. I was not so sure about "instanceof" since null is not an instance of anything, thus the "class.isAssignable()" suggestion, but I guess that wouldn't be a problem.

In my case the entities I loaded on h:selectOneMenu are already persisted. All I was trying to do was initializing the lists at the first usage and using them throughout the session without reaching back to database for these entities. I do not use these entities for any other purpose and they are not creatable in the application level. I guess we can say that they are hardcoded at database level.

The problem of HashSet(or HashMap) is troublesome for me. I guess I should decide what comparisons should I make in order to realize that two instances of MyEntity was in fact the same instance, whitout depending on id field; and then implement my hashCode() override accordingly..

Thanks for clarifying..