[
https://issues.apache.org/jira/browse/LANG-467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12642958#action_12642958
]
davey.jones edited comment on LANG-467 at 10/28/08 2:10 AM:
------------------------------------------------------------
The use of compareTo() in EqualsBuilder() is arguably wrong, however compareTo
is referenced from the BigDecimal javadoc as the alternative to equals for
cases where the scale is not relevant
In LANG-393 it was decided that EqualsBuidler should use the compareTo() method
to compare BigDecimals(), which is a nice convenience for those of use using
BigDecimals in conjunction with EqualsBuilder and who want 0 and 0.0 to be
considered equal.
However LANG-393 did not put the equivalent fix into the HashCodeBuilder so
this has made HashCodeBuilder and EqualsBuilder non-symmetric. It was actually
LANG-393 which added a feature to EqualsBuilder and at the same time created
this bug in HashCodeBuilder.
So as far as the EqualsBuilder is concerned 10.2 and 10.20 are equal, i.e. the
following evaluates to true.
{code}
new EqualsBuilder().append(new BigDecimal("10.2"), new
BigDecimal("10.20")).isEquals();
{code}
However when using these two values with HashCodeBuilder they actually give
different hashCodes(), the following evaluates to false.
{code}
new HashCodeBuilder(17, 37).append(new BigDecimal("10.2")).toHashCode() == new
HashCodeBuilder(17, 37).append(new BigDecimal("10.20")).toHashCode()
{code}
However the contract of hashCode() method for Object says that if two objects
are considered equal using their equals method then they must also generate the
same hashCode().
Of course this is true for BigDecimal class itself, even though it is somewhat
inconvenient.
MyPojo class as given in the test case above
* implements the equals() method as documented by EqualsBuilder
* implements the hashCode() method as documented by HashCodeBuilder
Despite following the documented approach for implementing equals and hashCode
the test case *proves* that MyPojo breaks the contract of hashCode(), the
following evaluates to true
{code}
myPojo1.equals(myPojo2)
{code}
However myPojo1 and myPojo2 generate different hashCodes(), the following
evaluates to false
{code}
myPojo1.hashCode() == myPojo2.hashCode()
{code}
was (Author: davey.jones):
The use of compareTo() in EqualsBuilder() is arguably wrong, however
compareTo is referenced from the BigDecimal javadoc as the alternative to
equals for cases where the scale is not relevant
In LANG-393 it was decided that EqualsBuidler should use the compareTo() method
to compare BigDecimals(), which is a nice convenience for those of use using
BigDecimals in conjunction with EqualsBuilder and who want 0 and 0.0 to be
considered equal.
However LANG-393 did not put the equivalent fix into the HashCodeBuilder so
this has made HashCodeBuilder and EqualsBuilder non-symmetric. It was actually
LANG-393 which added a feature to EqualsBuilder and at the same time created
this bug in HashCodeBuilder.
So as far as the EqualsBuilder is concerned 10.2 and 10.20 are equal, i.e. the
following evaluates to true.
{code}
new EqualsBuilder().append(new BigDecimal("10.2"), new
BigDecimal("10.20")).isEquals();
{code}
However when using these two values with HashCodeBuilder they actually give
different hashCodes(), the following evaluates to false.
{code}
new HashCodeBuilder(17, 37).append(new BigDecimal("10.2")) == new
HashCodeBuilder(17, 37).append(new BigDecimal("10.20"))
{code}
However the contract of hashCode() method for Object says that if two objects
are considered equal using their equals method then they must also generate the
same hashCode().
Of course this is true for BigDecimal class itself, even though it is somewhat
inconvenient.
MyPojo class as given in the test case above
* implements the equals() method as documented by EqualsBuilder
* implements the hashCode() method as documented by HashCodeBuilder
Despite following the documented approach for implementing equals and hashCode
the test case *proves* that MyPojo breaks the contract of hashCode(), the
following evaluates to true
{code}
myPojo1.equals(myPojo2)
{code}
However myPojo1 and myPojo2 generate different hashCodes(), the following
evaluates to false
{code}
myPojo1.hashCode() == myPojo2.hashCode()
{code}
> EqualsBuilder and HashCodeBuilder treat java.math.BigDecimal inconsistantly
> and break general contract of hashCode
> ------------------------------------------------------------------------------------------------------------------
>
> Key: LANG-467
> URL: https://issues.apache.org/jira/browse/LANG-467
> Project: Commons Lang
> Issue Type: Bug
> Affects Versions: 2.4
> Reporter: David Jones
> Priority: Minor
>
> A POJO with a BigDecimal field and equals() and hashCode() methods
> implemented using EqualsBuilder and HashCodeBuilder breaks the general
> contract of Object.hashCode();
> EqualsBuilder treats BigDecimal specially by comparing it using
> BigDecimal.compareTo() == 0 rather than BigDecimal.equals()
> Using BigDecimal.compareTo() ignores the scale of the BigDecimal()
> However the append(Object o) method of HashCodeBuilder uses
> BigDecimal.hashCode() in the case that o is a BigDecimal, which takes the
> scale into account when generating the hashCode.
> The following test case shows the problem!
> {code:title=TestApacheCommonsLangHashCodeBuilder.java|borderStyle=solid}
> // package declaration and imports not shown
> public class TestApacheCommonsLangHashCodeBuilder extends TestCase {
>
> public void testHashCode() {
> MyPojo myPojo1 = new MyPojo(new String("foo"), new
> BigDecimal("10.2"));
> MyPojo myPojo2 = new MyPojo(new String("foo"), new
> BigDecimal("10.20"));
>
> // equals method ignores the scale of the big decimal
> // so this test passes
> assertTrue(myPojo1.equals(myPojo2));
>
> // however in the case the equals returns true the
> // hashCode must be the same according to the contract
> // TEST FAILS AT THIS LINE
> assertEquals(myPojo1.hashCode(), myPojo2.hashCode());
> }
>
> private class MyPojo {
> private String name;
> private BigDecimal value;
>
> public MyPojo(String name, BigDecimal value) {
> this.name = name;
> this.value = value;
> }
>
> public String getName() {
> return name;
> }
> public BigDecimal getValue() {
> return value;
> }
> /**
> * equals method implemented using EqualsBuilder
> * as documented by apache commons
> */
> @Override public boolean equals(Object obj) {
> if(this == obj) {
> return true;
> }
>
> if(!(obj instanceof MyPojo)) {
> return false;
> }
>
> MyPojo other = (MyPojo) obj;
> return new EqualsBuilder()
> .append(name, other.getName())
> .append(value, other.getValue())
> .isEquals();
> }
>
> /**
> * hashCode method implemented using HashCodeBuilder
> * as documented by apache commons
> */
> @Override public int hashCode() {
> HashCodeBuilder hcb = new HashCodeBuilder(17, 31);
> return hcb
> .append(name)
> .append(value)
> .toHashCode();
> }
> }
> }
> {code}
> Note that the only reason I haven't provided a patch is because I could not
> think of one which I thought was reasonable.
> *Option 1*
> Always set the scale to some value and then get the hashCode()
> Example change in HashCodeBuilder.append(Object) add the following:
> {code}
> else if (object instanceof BigDecimal) {
> append(((BigDecimal) object).setScale(DEFAULT_BIGDECIMAL_SCALE,
> RoundingMode.DOWN).hashCode());
> }
> {code}
> However what is a reasonable scale for calculating this hashCode? I cannot
> see a reasonanble scale to choose, that depends on the circumstance
> *Option 2*
> stripTrailingZeros() before calculating the hashCode()
> Example change in HashCodeBuilder.append(Object) add the following:
> {code}
> else if (object instanceof BigDecimal) {
> append(((BigDecimal) object).stripTrailingZeros().hashCode());
> }
> {code}
> The performance of this method under different circumstances is not
> documented.
> *Option 3*
> Document the problem and propose that the client solves the problem.
> For example change HashCodeBuilder documentation as follows
> {code}
> /*
> * ...
> * public class Person {
> * String name;
> * int age;
> * boolean smoker;
> * BigDecimal netWorth;
> * ...
> *
> * public int hashCode() {
> * // you pick a hard-coded, randomly chosen, non-zero, odd number
> * // ideally different for each class
> * return new HashCodeBuilder(17, 37).
> * append(name).
> * append(age).
> * append(smoker).
> * // take special care when using BigDecimal as scale takes
> * // is included in the hashCode calculation breaking hashCode contract
> * // choose a scale which is reasonable for hashCode calculation
> * append(netWorth == null ? null : netWorth.setScale(2)).
> * toHashCode();
> * }
> * }
> * ...
> */
> {code}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
