[
https://issues.apache.org/jira/browse/LANG-467?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
David Jones updated LANG-467:
-----------------------------
Description:
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}
was:
A POJO with a BigDecimal field and equals() and hashCode() methods implemented
using EqualsBuilder and HashCodeBuilder break the general contract of
Object.hashCode();
EqualsBuilder treats BigDecimal special 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 appending the scale.
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 perormance 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}
typo fix and clarity improved
> 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.
