3

I was on #hibernate IRC and somebody shared the following (partial) pattern with me

@Entity
public MyEntity() {

  ... primary key, object properties, getters/setters go here ...    

  @Column(nullable=false)
  private int hashCode;

  public MyEntity() {
     hashCode += id;
  }

  private final Set<String> tags = Sets.newHashSet();

  public void addTag(String tag) {
     if(tags.add(tag)) {
         hashCode += tag.hashCode();
     }
  }

  public void removeTag(String tag) {
     if(tags.remove(tag) {
        hashCode -= tag.hashCode();
     }
  }

  public void hashCode() {
    return hashCode;
  }

  ... http://www.artima.com/lejava/articles/equality.html style equals ...
}

One could call this a "cached hashCode which is updated piecewise". (This is definitely NOT a "business key" pattern as some commenters seem to believe. The "business key" pattern requires a column with a uniqueness constraint on it, like a username, which is used for equality tests).

When used in JPA/Hibernate, it means that the @Entity can has similar advantages to "eq/hC with buisness [sic] key" from the JBoss Equals and HashCode article, yet behave the way a developer would expect any normal Javabean object to behave (i.e. without having to think about the object like a database row): before being persisted to the DB; after a Transaction in EAGER fetching modes; and at any time with LAZY fetch inside a Transaction or in EXTENDED mode.

However, ensuring that the hashCode is always updated correctly could be a real challenge.

Does anybody here have any experience with this pattern and could you share your discoveries (both positive and negative) about it? I'm extremely interested in Gotchas but I am not at all interested in comments which make claims that something is "bad" without solid arguments about why it is bad.

Note that I am aware of The JPA hashCode() / equals() dilemma, but I don't believe this pattern was actually covered in that discussion.

This pattern was originally suggested as a way to avoid a problem when loading nested Collections in @Entitys, such as encountered in Hibernate LazyInitializationException on find() with EAGER @ElementCollection.

UPDATE: some commenters are getting very passionate about the existing approaches. For the avoidance of doubt, I am just interested in the merits of this new pattern. For your reference, and to also ask you to stop saying how you believe equals/hashCode should be implemented, note that I have used the following pattern for my @Entitys for several years now:

@Id
private UUID id = UUID.randomUUID();

@Override
public boolean equals(Object obj) {
    if (this == obj)
        return true;
    if (!(obj instanceof MY_CLASS) || id == null)
        return false;
    MY_CLASS other = (MY_CLASS) obj;
    return id.equals(other.id);
}

@Override
public int hashCode() {
    Preconditions.checkNotNull(id, "id must be set before @Entity.hashCode can be called");
    return id.hashCode();
}

and I only recently tried something new to see if I really needed to have a separate method like this

public boolean hasSameProperties(Note other) {
    Preconditions.checkNotNull(other);
    if (this == other)
        return true;
    return Objects.equal(source, other.source)
            && Objects.equal(title, other.title)
            && Objects.equal(tags, other.tags)
            && Objects.equal(contents, other.contents);
}
Community
  • 1
  • 1
fommil
  • 5,757
  • 8
  • 41
  • 81
  • the implementation of equals is `this.businessKey == other.businessKey;`? – Affe Jul 30 '12 at 22:19
  • I don't get this at all. The hashcode will change as the object is mutated. How can that be a good idea? What use is it? – Tom Anderson Jul 30 '12 at 22:48
  • @Tom it saves fetching all fields from the DB in order to calculate a hashCode, so LAZY loading only happens for equals or field access. It also means that nested Collections don't cause load order problems in EAGER mode. – fommil Jul 31 '12 at 06:23
  • And it leaves you with a hashcode which changes over time, which is useless. – Tom Anderson Jul 31 '12 at 07:31
  • @fommil: Basing entities' `equals` on a collection??? That's like basing man's identity on the set of books he's borrowed. – maaartinus Sep 27 '12 at 21:39
  • @maaartinus to continue your analogy, you're saying that two libraries should be equal even if they don't hold the same books. If a `Person` data class contained a `Collection` (and many other things) then it is a reasonable choice that two instances should be equal if all their properties are equal including the collection of books and nature in which the books are stored (btw, `equals` is not the same as `==` identity in Java). For JPA managed data classes, good practice is to base `equals` on the `id` field. – fommil Oct 03 '12 at 17:30
  • @fommil: No, I'm saying that the *identity* of my home library doesn't change when I add a book. I'm also saying that mine and yours library are two *different* things even when they should happen to contain exactly the same books. And exactly this behavior can be achieved by using `id` based `equals`. – maaartinus Oct 03 '12 at 19:05
  • If that is what you think is happening here, I'll update the question to clarify. Nobody said anything about changing the PrimaryId (I don't think it's even possible to do that). Also, I'm fully aware about the id as equals pattern: I'll assume you skimmed that bit of the question. – fommil Oct 04 '12 at 07:04

2 Answers2

1

This business key isn't unique, so it seems like a poor choice for equals().

Business key 1002 has tag 500 and tag -502 added to it, business key 995 has tag 3 and tag 2 added to it? Are those objects really meant to be equal?

The relatively low risk of collisions for 32 bit numbers may be tolerable for whatever purpose your particular entity serves, but to refer to something as a 'Pattern' one would like for it to actually be correct, not just arbitrarily unlikely to fail for a given data size.

Affe
  • 47,174
  • 11
  • 83
  • 83
  • Affe, I think your point is so important that I now believe the person who shared this pattern was only talking about hashCode, not equals. Question updated to reflect the pattern. – fommil Jul 31 '12 at 06:19
0

I've written, rewritten, and deleted two or three long answers explaining why i think this is a bad idea, but ultimately, all any of them were basic explanations of what object identity is, and i'm sure you understand that.

My response to this pattern is that it just doesn't solve any problem which needs solving.

The solution to the alleged "hashCode/equals problem" is incredibly simple. Give your entities an identifier when you create them. Persist it. Base equals and hashCode on it. Do not rely on an identifier created by the database or persistence layer at insertion. This is by now a positively ancient principle.

As an aside, for entities, it is never right to base either equals or hashCode on mutable fields. That is not how equality works for entities. Entities are not just a bag of state, they are an identity to which some state happens to be attached. Equality and hashcode must be based on the identity, not the state.

Tom Anderson
  • 46,189
  • 17
  • 92
  • 133
  • You're declaring that "a hashcode which changes over time ... is useless". I disagree! It would allow entities (which are fully fetched from a DB, and no longer managed) to be compared with other unmanaged entities as if they were JavaBeans - entirely appropriate for small to medium size applications. You are declaring that equality should always be based on the 'id', but that approach (which I have used in the past, and have used to date) has its drawbacks. I'm trying to explore a different pattern here and I'm interested in hearing people's experiences of it. – fommil Jul 31 '12 at 09:14
  • @fommil: I think a hashcode for an entity which changes over time is a flat-out bad idea. I believe you're conflating the idea of entities and value objects, and i also think that is a flat-out bad idea. But hey, try it out, see how it goes. Just stay away from my codebase! – Tom Anderson Jul 31 '12 at 17:02
  • I'm not asking for opinion, I'm asking for experience from anyone who has used this pattern. Or any non-obvious gotchas - you've just essentially thrown my own references back at me. I'm just wanting to investigate a new approach because I'm open minded and I believe in reassessing the situation once in a while. I don't intend on going anywhere near your codebase, what a weird thing to say. – fommil Jul 31 '12 at 17:33
  • @Tom Anderson: +2 (if I could). IMHO you're perfectly right with everything and I really don't get why people want to do anything else (anything else is slower and can break anytime). Especially why the Hibernate guys recommend such strange things. – maaartinus Sep 27 '12 at 22:12
  • @maaartinus so you don't like people trying out new things? Well that's exactly how new stuff doesn't get discovered. Try something new and either (a) fail and go back to the original or (b) find that the new thing is actually better. I actually fully agree with Tom's points (although I don't think they are constructive here) and you will see the pattern I use in my codebase reflects this. This question was born from a suggestion on the hibernate IRC channel. I have since discovered that it was just one guy's idea and its a flop. Big deal. – fommil Oct 03 '12 at 17:40
  • @fommil: Exploring new things is nice and important, but this "business key" is an old anti-pattern. So I think it's good to advise everybody against it, although I could imagine there're rare cases where it might make sense. – maaartinus Oct 03 '12 at 19:00