views:

611

answers:

2

Given the following:

@Entity
public class Parent implements Serializable {
  @Id
  private Long id;
  // mapped ManyToOne below...
  private List<Child> children = new ArrayList<Child>();

  ...

}

Is it a bad practice to have Parent.equals() and Parent.hashCode() use only id? I understand that Child.equals() and Child.hashCode() should use a immutable set of attributes for a "natural key" for them to be correctly managed by Parent. However, if Parent is always a top-level object (ie it's never the inverse side of any association), is there anything wrong with using only id?

Are there any unwanted effects that could manifest themselves by doing this? I am guessing that maybe if I do this, that when I add a child (or remove), Hibernate won't be able to tell that Parent has changed (and needs to be updated in DB)? In this case, should I use the children property for Parent.equals() and Parent.hashCode()?

I am asking because the Hibernate docs explicity say not to use the @Id property for a "natural key"...

+5  A: 

The primary problem with using ID as a equals and hashCode basis is unpersisted objects. These objects presumably all start with the same ID and this can't properly be compared for equality. Even if you never put the objects in a collection, if they're exposed via an API and someone else can create instance of them and put them into a collection then you've opened yourself up to some nasty bugs.

Jherico
I understand what you are saying, but technically if id is zero, then they really ARE equal: they are both unpersisted Parent objects. For my above example with only those two properties (id and children), how would you suggest implementing equals() and hashCode()? Would I need to introduce more properties?
GreenieMeanie
Even if there is no Hibernate involved, can you ever allow mutators on fields that are used for equals() and hashCode()? Someone could put an instance in a HashSet, change a field that affects the hash code, and in that way break the set. But forbidding it seems a little harsh. I'm leaning towards that you have to consider each case separately and weigh the pros and cons against each other...
waxwing
@Greenie: If I'm importing data from another source I might create a whole graph of new objects that I have to store somewhere before persisting them and flushing hibernate. Just because they have ID 0 doesn't mean they are equal, it means they don't have an ID yet is all.
Jherico
@Waxwing: yes, if you put an object into a hash set and then change an attribute that causes its hash to change, then you've broken set.contains(). The correct response is either a) don't do that or b) be sure if you do that you know the consequences. Its the price you pay for working with non-immutable objects.
Jherico
@Jherico: very good point. I am still waiting for a suggestion on how to implement it properly, as well as an answer to the question of whether or not I should use the children property in equals() and hashCode().
GreenieMeanie