You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Felix Knecht <fe...@otego.com> on 2010/05/15 10:35:46 UTC

Equals and HashCode

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

We have a quite a lot of the following findings:

HE: Class defines equals() and uses Object.hashCode()
(HE_EQUALS_USE_HASHCODE)
See [1], ...

Do we consider them all as false positives or should we override the
hashCode function?

Felix

[1]
http://findbugs.sourceforge.net/bugDescriptions.html#HE_EQUALS_USE_HASHCODE
[2]
http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Object.html#hashCode%28%29
[3]
http://bytes.com/topic/java/insights/723476-overriding-equals-hashcode-methods
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvuXOIACgkQ2lZVCB08qHFDzACg56XbS76zk694htt1owwRBAsu
z4wAoNIFr+Ed90uIwQ7H2eyfWlz3NEEa
=QIJQ
-----END PGP SIGNATURE-----

Re: Equals and HashCode

Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/17/10 15:30, Emmanuel Lecharny wrote:
> On 5/17/10 1:44 PM, Felix Knecht wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>   
>>> IMO, writing a decent HashCode() method is simple. No need to use this
>>> HashCodeBuilder that don't lnow how to deal with Object (it simply add
>>> the object.hashcode() to the computed hash code, which is bad).
>>>      
>> I don't see whats bad doing so. The rule is "if two objects are equal,
>> then their hashCode values must be equal as well". As a very bad
>> solution we can just return 1111 in every case :(
>>    
> It's bad as it carries the semantic, not the logic. In other words, ypu
> may have two objects considered as different when they are equals, just
> because you were too lazzy to implement the hashcode method. I give you
> an example : comparing two Entry instance in the server is just about
> comparing their DN, nothing else. If you call the HashcodeBuilder on it,
> you will get errors if the Attributes are the same, but stored in a
> different order, which is not expected.

I see. Thanks for explanation.

> 
> Of course, this is a case by case issue.
> 
> What I mean here is that hashcode computation must be contextual.
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvxRbgACgkQ2lZVCB08qHHqZACcCx+jp4oGRqouKM975ZQTEwaS
0twAniZ3wfWrliOofo/p9euHZQJ/6IVW
=821N
-----END PGP SIGNATURE-----

Re: Equals and HashCode

Posted by Emmanuel Lecharny <el...@gmail.com>.
On 5/17/10 1:44 PM, Felix Knecht wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>    
>> IMO, writing a decent HashCode() method is simple. No need to use this
>> HashCodeBuilder that don't lnow how to deal with Object (it simply add
>> the object.hashcode() to the computed hash code, which is bad).
>>      
> I don't see whats bad doing so. The rule is "if two objects are equal,
> then their hashCode values must be equal as well". As a very bad
> solution we can just return 1111 in every case :(
>    
It's bad as it carries the semantic, not the logic. In other words, ypu 
may have two objects considered as different when they are equals, just 
because you were too lazzy to implement the hashcode method. I give you 
an example : comparing two Entry instance in the server is just about 
comparing their DN, nothing else. If you call the HashcodeBuilder on it, 
you will get errors if the Attributes are the same, but stored in a 
different order, which is not expected.

Of course, this is a case by case issue.

What I mean here is that hashcode computation must be contextual.

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com



Re: Equals and HashCode

Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> IMO, writing a decent HashCode() method is simple. No need to use this
> HashCodeBuilder that don't lnow how to deal with Object (it simply add
> the object.hashcode() to the computed hash code, which is bad).

I don't see whats bad doing so. The rule is "if two objects are equal,
then their hashCode values must be equal as well". As a very bad
solution we can just return 1111 in every case :(

What's bad in returning a sum of the has codes of each object hash code
used in the in the equals method?

> 
> This hashCodeBuilder() method is only called in one place in the whole
> code (ArrayUtils), and I'm not even sure it's useful.
> 
> Anyway, just use the commons-lang one in ArrayUtils, and delete this
> method from the code forever :)
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvxLAoACgkQ2lZVCB08qHGbaACg3Hsc8YcKoBKaRq7eQBFn0Mh5
/k0AoPARWKSU5OJZFKn/t0YaG90D+Q+L
=PPlF
-----END PGP SIGNATURE-----

Re: Equals and HashCode

Posted by Emmanuel Lecharny <el...@gmail.com>.
On 5/16/10 11:37 PM, Felix Knecht wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>    
>> Definitively not a false positive. Implementing the equals() method does
>> not works if we don't implement hashcade() at the same time.
>>      
> IMO we can use for most HashCodeBuilder [1] and appending the same
> fields used in the equals method.
> Do you think this is a way to go or does this end up in performance issues?
>    

IMO, writing a decent HashCode() method is simple. No need to use this 
HashCodeBuilder that don't lnow how to deal with Object (it simply add 
the object.hashcode() to the computed hash code, which is bad).

This hashCodeBuilder() method is only called in one place in the whole 
code (ArrayUtils), and I'm not even sure it's useful.

Anyway, just use the commons-lang one in ArrayUtils, and delete this 
method from the code forever :)

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com



Re: Equals and HashCode

Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> Definitively not a false positive. Implementing the equals() method does
> not works if we don't implement hashcade() at the same time.

IMO we can use for most HashCodeBuilder [1] and appending the same
fields used in the equals method.
Do you think this is a way to go or does this end up in performance issues?


[1]
http://commons.apache.org/lang/api-release/org/apache/commons/lang/builder/HashCodeBuilder.html

Felix
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvwZYsACgkQ2lZVCB08qHEhMQCgwEfA9rcjzUIJw1SXqv6kUEiO
QzsAn1+4At1CrowV7INrfV5gm8n8FD7m
=Pmc3
-----END PGP SIGNATURE-----

Re: Equals and HashCode

Posted by Alex Karasulu <ak...@apache.org>.
On Mon, May 17, 2010 at 12:10 AM, Emmanuel Lecharny <el...@gmail.com>wrote:

> On 5/16/10 10:52 PM, Felix Knecht wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> The code seems to be the same like in the implementations of Apache
>> Commons-Lang Version 2.1.
>>
>> If nobody objects I'll drop the shared class and use the from the
>> commons-lang project (it's anyway already a dependency)
>>
>>
> Do that.
>
> The reason why the code is duplicated from commons-lang was that Alex
> wanted to limit the dependencies to external jars as much as possible at the
> beginning of the project, as it was intended to be embedded. So he simply
> copied chunks of code from those projects into ADS.
>
> Now that we have tens of dependencies, this is a lost battle anyway, better
> benefit from the bug fixes applied to those dependencies !
>
>
Yep yep !!!

-- 
Alex Karasulu
My Blog :: http://www.jroller.com/akarasulu/
Apache Directory Server :: http://directory.apache.org
Apache MINA :: http://mina.apache.org
To set up a meeting with me: http://tungle.me/AlexKarasulu

Re: Equals and HashCode

Posted by Emmanuel Lecharny <el...@gmail.com>.
On 5/16/10 10:52 PM, Felix Knecht wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> The code seems to be the same like in the implementations of Apache
> Commons-Lang Version 2.1.
>
> If nobody objects I'll drop the shared class and use the from the
> commons-lang project (it's anyway already a dependency)
>    
Do that.

The reason why the code is duplicated from commons-lang was that Alex 
wanted to limit the dependencies to external jars as much as possible at 
the beginning of the project, as it was intended to be embedded. So he 
simply copied chunks of code from those projects into ADS.

Now that we have tens of dependencies, this is a lost battle anyway, 
better benefit from the bug fixes applied to those dependencies !

Thanks !

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com



Re: Equals and HashCode

Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The code seems to be the same like in the implementations of Apache
Commons-Lang Version 2.1.

If nobody objects I'll drop the shared class and use the from the
commons-lang project (it's anyway already a dependency)

> In Directory Shared exist a class
> org.apache.directory.shared.ldap.util.HashCodeBuilder
> 
> I there something special about it compared to the one from the Apache
> Commons project [1] (it does quite the same)? If not why not drop the
> one from Shared and use the Commons implementation?
> 
> Felix
> 
> 
> [1]
> http://commons.apache.org/lang/api/org/apache/commons/lang/builder/HashCodeBuilder.html
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvwWyMACgkQ2lZVCB08qHFWzgCgyTn27sZDFeNPtzpj+OC/LBiM
WF4AnRHoaILLKNUjTTfqipeAoTxxbl/L
=TJuC
-----END PGP SIGNATURE-----

Re: Equals and HashCode

Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> Definitively not a false positive. Implementing the equals() method does
> not works if we don't implement hashcade() at the same time.

In Directory Shared exist a class
org.apache.directory.shared.ldap.util.HashCodeBuilder

I there something special about it compared to the one from the Apache
Commons project [1] (it does quite the same)? If not why not drop the
one from Shared and use the Commons implementation?

Felix


[1]
http://commons.apache.org/lang/api/org/apache/commons/lang/builder/HashCodeBuilder.html

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvvp4UACgkQ2lZVCB08qHEN/ACePRVx3OXCZy7st3MN4h75p6RI
zNYAn2JYIQKAZnkJag2TgSedcNJfv1OY
=7xJ6
-----END PGP SIGNATURE-----

Re: Equals and HashCode

Posted by Emmanuel Lecharny <el...@gmail.com>.
On 5/15/10 10:35 AM, Felix Knecht wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> We have a quite a lot of the following findings:
>
> HE: Class defines equals() and uses Object.hashCode()
> (HE_EQUALS_USE_HASHCODE)
> See [1], ...
>
> Do we consider them all as false positives or should we override the
> hashCode function?
>    
Definitively not a false positive. Implementing the equals() method does 
not works if we don't implement hashcade() at the same time.

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com