equals() / hashCode(): Is there *any* non-broken approach?

RobJellinghaus · **Joined:** Thu Jan 08, 2004 6:17 pm **Posts:** 278

Actually, some of Paul's suggestions about "hollow object"-like behavior in Hibernate were never resolved nor reflected in the wiki page you cite. But that's because Hibernate simply doesn't support these kinds of semantics in its bytecode generation.

The wiki page accurately describes all the straightforward approaches to this issue in Java and hence in Hibernate; many of Paul's proposals were pushing significantly beyond Hibernate and hence are not addressed in the wiki.

Cheers!
Rob

eepstein · **Posted:** Wed Apr 21, 2004 3:18 pm

The equals() method on CatImpl shown at the last posting on the first page will cause an infinite loop.

RobJellinghaus · **Joined:** Thu Jan 08, 2004 6:17 pm **Posts:** 278

eepstein wrote:

The equals() method on CatImpl shown at the last posting on the first page will cause an infinite loop.

No, it won't. Why exactly do you think it will?

Cheers,
Rob

melquiades · **Posted:** Wed Apr 21, 2004 3:54 pm

RobJellinghaus wrote:

eepstein wrote:

The equals() method on CatImpl shown at the last posting on the first page will cause an infinite loop.

No, it won't. Why exactly do you think it will?

No, no, he's right: if you compare two objects whose equalityObjects are both *this* (including comparing an unpersisted object to itself), it will go into an infinite recursion.

It should have a boolean called "useInstanceEquality" which gets set to true if ID is null, then either call super.equals() or id.equals(), depending on which is set. Something like:

Code:

// WARNING: untested code!

public CatImpl extends Cat {
    private Long id;
    private boolean comparedBeforeIdAssigned;
   
    private boolean usingInstanceEquality() {
        if(id == null)
           comparedBeforeIdAssigned = true;
        return comparedBeforeIdAssigned;
    }
   
    public boolean equals(Object that) {
        if(that instanceof CatImpl && usingInstanceEquality()) {
            return this == that;
        } else if (that instanceof Cat) {
            return ((Cat) that).getId().equals(getId());
        } else {
            return false;
        }
    }
   
    public int hashCode() {
        return usingInstanceEquality() ? super.hashCode() : getId().hashCode();
    }
}

melquiades · **Posted:** Wed Apr 21, 2004 3:57 pm

I also notice that the second branch of the if() in equals() isn't null-safe. Somebody ought to test this code sometime! :)

Code:

// WARNING: untested code!

public CatImpl extends Cat {
    private Long id;
    private boolean comparedBeforeIdAssigned;
   
    private boolean usingInstanceEquality() {
        if(id == null)
           comparedBeforeIdAssigned = true;
        return comparedBeforeIdAssigned;
    }
   
    public boolean equals(Object that) {
        if(that instanceof CatImpl && usingInstanceEquality()) {
            return this == that;
        } else if (that instanceof Cat) {
            Object otherId = ((Cat) that).getId();
            return otherId != null && otherId.equals(getId());
        } else {
            return false;
        }
    }
   
    public int hashCode() {
        return usingInstanceEquality() ? super.hashCode() : getId().hashCode();
    }
}

[/quote]

emmanuel · **Posted:** Thu Apr 22, 2004 3:13 am

melquiades wrote:

I also notice that the second branch of the if() in equals() isn't null-safe. Somebody ought to test this code sometime! :)

You should consider reading the JLS before suchaffirmation.

Quote:

At run time, the result of the instanceof operator is true if the value of the RelationalExpression is not null and the reference could be cast (

_________________
Emmanuel

immanuel · **Joined:** Tue Jan 13, 2004 4:57 am **Posts:** 83

Quote:

emmanuel wrote:

melquiades wrote:

I also notice that the second branch of the if() in equals() isn't null-safe. Somebody ought to test this code sometime! :)

You should consider reading the JLS before suchaffirmation.

Well, the second part of it isn't so wrong: the new hbm2java gives a nullpointerexception on startup if you don't provide a config file, in addition to the equals problem...

michael · **Posted:** Thu Apr 22, 2004 4:09 am

It's okay man, we know about the bug, no need to post it into every thread about hbm2java or equals you can find.

immanuel · **Joined:** Tue Jan 13, 2004 4:57 am **Posts:** 83

michael wrote:

It's okay man, we know about the bug, no need to post it into every thread about hbm2java or equals you can find.

Sorry you take offence of it, but you don't know how being bugged by these little things hurts when using hibernate in a potentially "hostile-to-hibernate" environment: I've been fighting for weeks in order to push the hibernate choice, and I now work with a group of consultants which could use every bug in tools to tell the management "we are late, it's hibernate's fault: there were two bugs in it, we had to patch it and so on". You know it can be damn hard to demonstrate it was not "hibernate" but one of the accompanying tools.

immanuel · **Joined:** Tue Jan 13, 2004 4:57 am **Posts:** 83

immanuel wrote:

michael wrote:

It's okay man, we know about the bug, no need to post it into every thread about hbm2java or equals you can find.

Sorry you take offence of it, but you don't know how being bugged by these little things hurts when using hibernate in a potentially "hostile-to-hibernate" environment: I've been fighting for weeks in order to push the hibernate choice, and I now work with a group of consultants which could use every bug in tools to tell the management "we are late, it's hibernate's fault: there were two bugs in it, we had to patch it and so on". You know it can be damn hard to demonstrate it was not "hibernate" but one of the accompanying tools.

Moreover, rethinking at where I've talked about it previously, it was in a thread where a user asked how to circumvent another bug, and (after helping him) I was simply warning him that he should take care of another one and he could patch hbm2java as well. So I guess the right answer is "sorry you take offence at it, but gimme a break".

melquiades · **Posted:** Thu Apr 22, 2004 3:57 pm

emmanuel wrote:

You should consider reading the JLS before suchaffirmation.

You should consider reading the actual post you're replying to before such snarky pedantry. (Yes, thank you, I know how instanceof works. Sheesh.)

The code broke if ((Cat) that).getId() returns null, not if that itself is null.

emmanuel · **Posted:** Thu Apr 22, 2004 4:03 pm

melquiades wrote:

You should consider reading the actual post you're replying to before such snarky pedantry. (Yes, thank you, I know how instanceof works. Sheesh.)

The code broke if ((Cat) that).getId() returns null, not if that itself is null.

My bad, I read your implem, not the offending one.

eepstein · **Posted:** Thu Apr 22, 2004 6:52 pm

melquiades wrote:

My cat example has a serious problem; that if-check that throws the exception is bogus, and just shouldn't be there at all. I'll post a revised version if/when we actually get it tested & working.

There's another issue. The redefinition of equals() breaks symmetry.

Suppose you have two objects A and B of the same Cat class. Suppose A is persistent and so compares via Id, while B is new, so compares via object Identity. Now B gets persisted. It still compares via Object identity so when asking if B.equals(A) this is false. But, under some circumstances mightn't you have A.equals(B) -- which will compare via Ids -- perhaps return a different result under some circumstances?

RobJellinghaus · **Joined:** Thu Jan 08, 2004 6:17 pm **Posts:** 278

eepstein wrote:

Suppose you have two objects A and B of the same Cat class. Suppose A is persistent and so compares via Id, while B is new, so compares via object Identity. Now B gets persisted.

Mel addressed this issue in this post:

http://forum.hibernate.org/viewtopic.php?p=2191778#2191778

The key is to ask in what session are A and B? If A was loaded from a given session, then generally you would not newly create a B that would be presumed equal to A. More likely if you are creating a new object, you are saving it in a given session -- and then if you reload it in that session, the session cache will resolve it to the same instance, which will compare with the same equality behavior.

In other words, you wouldn't have a new B that you would be comparing to a persistent A -- at least, not in the same session. If you tried to detach A and/or B and later reattach them to a new session, then yes, you could get bad behavior, as Mel's post (that I cite above) makes clear.

Cheers!
Rob

eepstein · **Posted:** Thu Apr 22, 2004 8:25 pm

I too am looking for a (or a few) recommended solutions. The Hibernate approach says "don't override equals()/hashCode()". If that works for all cases then I guess it's settled. But in that case why does the new (2.1) hbm2java still default to generating equals()/hashCode() for compound keys?

And, moreover, how does not overriding eq/hc play with trying to use objects in a disconnected fashion? In order for concurrency to work in a web app we need to use disconnected objects. Are there rules for doing so when eq/hc is not overridden. Will session.lock(obj, NO_LOCK); still do the trick?