equals and hashCode methods

[Adam Hardy <ad...@cyberspaceroad.org>]

I put a superclass on all my entities for a couple of general properties that 
they all share, and now I'm considering putting my equals() and hashCode() 
methods into the superclass as well, with reflection to loop over the array of 
child methods, calling whatever POJO getters are present.

The advantage in terms of not needing to maintain the equals, hashCode and 
toString methods on every entity is quite attractive, but I'm worried about

(a) performance
(b) any nasty surprises it might cause in JPA entity management

With (a), performance will be hit if an entity has large collections of 
one-to-many or many-to-many related entities. I might be able to get around 
quite easily though.

With (b) it seems more problematic, for instance calling the equals() might get 
stuck in an endless loop if I have any circular relationships in my model, which 
would otherwise be benign if only loaded by lazy loading.

Sorry that this isn't directly an OpenJPA question.

This is the kind of thing I'm thinking of:

public boolean equals(Object object) {
     if (this == object) return true;
     if (object == null) return false;
     if (!(object instanceof TradeHistory)) return false;
     TradeHistory other = (TradeHistory) object;
     boolean equal = false;
     Method[] methods = this.getClass().getMethods();
     for (int i = 0; i < methods.length; i++) {
         Method method = methods[i];
         if (method.getName().equalsIgnoreCase("GETCLASS")) continue;
         if ((method.getName().startsWith("get"))
             && (method.getParameterTypes().length == 0)) {
             try {
                 Method otherMethod =
                     other.getClass().getMethod(method.getName(),
                         new Class[] {});
                 equal &=
                     isEqual(method.invoke(this, new Object[] {}), otherMethod
                         .invoke(other, new Object[] {}));
             }
             catch (Exception e) {
             }
         }
     }
     return equal;
}

Re: equals and hashCode methods

[Adam Hardy <ad...@cyberspaceroad.org>]

I already use the commons-lang equalsBuilder and hashCodeBuilder, they are good. 
But they don't reduce the maintenance problem.

 From a safety point of view, there is no way to check programmatically (i.e. in 
Maven reporting) that a developer has correctly coded equals() and hashCode() 
and the result of laziness there could be spurious, hard-to-reproduce errors in 
a live environment.

So I now see these options:

(1) invent a test that really tests the hashCode() method - I can see this would 
take alot of thought. Perhaps such a test already exists on the net somewhere. 
In a more rigorous environment I would want to do this /whatever/.

(2) write a superclass hashCode() algorithm that ignores collections, sets and 
lists of the child, which would ensure that performance is not hit so badly.

(3) make sure hashCode() is checked manually at code review time (but code 
reviews only happen 1 in 5 times when they are meant to in my experience).

(4) your idea for an abstract method. This does actually seem the most appealing 
at this point. It could return an array of method names to invoke for equals() 
and hashCode()

I don't get your suggestion for caching. What would you cache exactly?

Regards
Adam



Kristof Jozsa on 27/03/08 09:22, wrote:
> I don't think that's a good idea long-term. Equals/hashcode usually targets the
> smallest set of business properties which can identify an object. Also, while
> reflection got much faster these days, heavy performance issues might still
> arise as especially hashcode() is called a lot.
> 
> You might further develop your idea with using EqualsBuilder/HashcodeBuilder
> from Jakarta Commons Lang, adding an abstract method defining the essencial
> properties to include in the generated equals/hashcode and add caching to
> hashcode values, but still, understanding and writing a proper equals/hashcode
> manually looks a much easier/safer path for me.
> 
> Adam Hardy wrote:
>> I put a superclass on all my entities for a couple of general properties
>> that they all share, and now I'm considering putting my equals() and
>> hashCode() methods into the superclass as well, with reflection to loop
>> over the array of child methods, calling whatever POJO getters are present.
>>
>> The advantage in terms of not needing to maintain the equals, hashCode
>> and toString methods on every entity is quite attractive, but I'm
>> worried about
>>
>> (a) performance
>> (b) any nasty surprises it might cause in JPA entity management
>>
>> With (a), performance will be hit if an entity has large collections of
>> one-to-many or many-to-many related entities. I might be able to get
>> around quite easily though.
>>
>> With (b) it seems more problematic, for instance calling the equals()
>> might get stuck in an endless loop if I have any circular relationships
>> in my model, which would otherwise be benign if only loaded by lazy
>> loading.
>>
>> Sorry that this isn't directly an OpenJPA question.
>>
>> This is the kind of thing I'm thinking of:
>>
>> public boolean equals(Object object) {
>>     if (this == object) return true;
>>     if (object == null) return false;
>>     if (!(object instanceof TradeHistory)) return false;
>>     TradeHistory other = (TradeHistory) object;
>>     boolean equal = false;
>>     Method[] methods = this.getClass().getMethods();
>>     for (int i = 0; i < methods.length; i++) {
>>         Method method = methods[i];
>>         if (method.getName().equalsIgnoreCase("GETCLASS")) continue;
>>         if ((method.getName().startsWith("get"))
>>             && (method.getParameterTypes().length == 0)) {
>>             try {
>>                 Method otherMethod =
>>                     other.getClass().getMethod(method.getName(),
>>                         new Class[] {});
>>                 equal &=
>>                     isEqual(method.invoke(this, new Object[] {}),
>> otherMethod
>>                         .invoke(other, new Object[] {}));
>>             }
>>             catch (Exception e) {
>>             }
>>         }
>>     }
>>     return equal;
>> }

Re: equals and hashCode methods

[Kristof Jozsa <dy...@ond.vein.hu>]

Hi Adam,

I don't think that's a good idea long-term. Equals/hashcode usually targets the
smallest set of business properties which can identify an object. Also, while
reflection got much faster these days, heavy performance issues might still
arise as especially hashcode() is called a lot.

You might further develop your idea with using EqualsBuilder/HashcodeBuilder
from Jakarta Commons Lang, adding an abstract method defining the essencial
properties to include in the generated equals/hashcode and add caching to
hashcode values, but still, understanding and writing a proper equals/hashcode
manually looks a much easier/safer path for me.

my 2c,
Kristof

Adam Hardy wrote:
> I put a superclass on all my entities for a couple of general properties
> that they all share, and now I'm considering putting my equals() and
> hashCode() methods into the superclass as well, with reflection to loop
> over the array of child methods, calling whatever POJO getters are present.
> 
> The advantage in terms of not needing to maintain the equals, hashCode
> and toString methods on every entity is quite attractive, but I'm
> worried about
> 
> (a) performance
> (b) any nasty surprises it might cause in JPA entity management
> 
> With (a), performance will be hit if an entity has large collections of
> one-to-many or many-to-many related entities. I might be able to get
> around quite easily though.
> 
> With (b) it seems more problematic, for instance calling the equals()
> might get stuck in an endless loop if I have any circular relationships
> in my model, which would otherwise be benign if only loaded by lazy
> loading.
> 
> Sorry that this isn't directly an OpenJPA question.
> 
> This is the kind of thing I'm thinking of:
> 
> public boolean equals(Object object) {
>     if (this == object) return true;
>     if (object == null) return false;
>     if (!(object instanceof TradeHistory)) return false;
>     TradeHistory other = (TradeHistory) object;
>     boolean equal = false;
>     Method[] methods = this.getClass().getMethods();
>     for (int i = 0; i < methods.length; i++) {
>         Method method = methods[i];
>         if (method.getName().equalsIgnoreCase("GETCLASS")) continue;
>         if ((method.getName().startsWith("get"))
>             && (method.getParameterTypes().length == 0)) {
>             try {
>                 Method otherMethod =
>                     other.getClass().getMethod(method.getName(),
>                         new Class[] {});
>                 equal &=
>                     isEqual(method.invoke(this, new Object[] {}),
> otherMethod
>                         .invoke(other, new Object[] {}));
>             }
>             catch (Exception e) {
>             }
>         }
>     }
>     return equal;
> }