Immuatable Object with "normalized" BigDecimal field.

[sbernard31]

Reading BigDecimal equality, I understand that one possible way to solve equals/hashcode issue with BigDecimal fields is to normalized field at object constructor of immutable class.

This ☝️ makes sense to me but I didn't understand why the library complains when we do that with something like :

BigDecimal equality: object does not equal a copy of itself where BigDecimal field b has a value that is equal using compareTo: 0 compared to 0.0
If these values should be considered equal then use compareTo rather than equals for this field.
If these values should not be considered equal, suppress Warning.BIGDECIMAL_EQUALITY to disable this check.

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
(EqualsVerifier 3.15.3, JDK 1.8.0_412 on Linux)

I totally understand that for now I need to suppress : Warning.BIGDECIMAL_EQUALITY to disable this check.

But as my code behave correctly, equalsVerifier should be able to test it ?

Here a code example :

The class to test :

public static class ClassWithBigDecimalField {
    private final BigDecimal b;

    public ClassWithBigDecimalField(BigDecimal b) {
        this.b = b.stripTrailingZeros();
    }

    @Override
    public final int hashCode() {
        return Objects.hash(b);
    }

    @Override
    public final boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (!(obj instanceof ClassWithBigDecimalField))
            return false;
        ClassWithBigDecimalField other = (ClassWithBigDecimalField) obj;
        return Objects.equals(b, other.b);
    }

    @Override
    public String toString() {
        return String.format("ClassWithBigDecimalField [b=%s]", b);
    }
}

Some tests :

@Test
public void manualTest() {

    BigDecimal referenceField = new BigDecimal("1.12322");
    // same number with different scale
    BigDecimal changedField = referenceField.setScale(referenceField.scale() + 1, RoundingMode.UNNECESSARY);

    ClassWithBigDecimalField referenceInstance = new ClassWithBigDecimalField(referenceField);
    ClassWithBigDecimalField changedInstance = new ClassWithBigDecimalField(changedField);

    // check equals and hashcode :this is OK ✅
    assertEquals(referenceInstance, changedInstance);
    assertEquals(referenceInstance.hashCode(), changedInstance.hashCode());
}

@Test
public void equalsVerifierTest() {
    // this is OK ✅
    EqualsVerifier.forClass(ClassWithBigDecimalField.class).suppress(Warning.BIGDECIMAL_EQUALITY).verify();

    // this is KO ❌
    EqualsVerifier.forClass(ClassWithBigDecimalField.class).verify();
}

So why equalsVerifier library does not generate a test like the manual one above ?
Maybe because of some Java Reflection Limitation ?

[jqno]

The answer is in the first line of the page you linked:

Be aware that BigDecimal fields with values such as 1, 1.0, 1.00, … are not considered equal.

In other words:

var a = new BigDecimal("1");
var b = new BigDecimal("1.0");
a.equals(b); // returns false!
a.compareTo(b) == 0 // returns true

Because of this strange behavior, the general advice is to use compareTo on BigDecimals rather than equals when you're writing an equals method. EqualsVerifier detects this, and that's the error you're seeing.

So, if you intended 1 and 1.0 to be equal in the context of your class, you should probably use compareTo on those BigDecimals, rather than equals.
If you intended these values to be not equal to each other, then you can suppress the warning.

[sbernard31]

Either my english is very bad (which is totally possible 😅) OR maybe you read me too fast 🤷 .

I'm aware of what you explain here, please read my comment again. 🙏

In your documentation, you talk about 3 solution :

1. do not use BigDecimal
2. use compareTo in equals
3. normalize BigDecimal before to store it in immutable object.

I'm talking about solution 3.

[jqno]

I totally misunderstood you the first time, sorry about that!

You're completely right, EqualsVerifier still wants you to suppress the warning, if you normalize the bigdecimals when they come in. I guess I never properly tested that scenario when I wrote the docs... Unfortunately, there's little I can do about it. EqualsVerifier has no way of seeing that you normalized the BigDecimal, so the suppression is still necessary.

I will update the error message and the documentation to make this more clear.

[sbernard31]

For immutable class, I have in mind that that EqualsVerifier could create instance using constructors then test for equals ?
like I did at :

@Test
public void manualTest() {

    BigDecimal referenceField = new BigDecimal("1.12322");
    // same number with different scale
    BigDecimal changedField = referenceField.setScale(referenceField.scale() + 1, RoundingMode.UNNECESSARY);

    ClassWithBigDecimalField referenceInstance = new ClassWithBigDecimalField(referenceField);
    ClassWithBigDecimalField changedInstance = new ClassWithBigDecimalField(changedField);

    // check equals and hashcode :this is OK ✅
    assertEquals(referenceInstance, changedInstance);
    assertEquals(referenceInstance.hashCode(), changedInstance.hashCode());
}

Do you think this could be something doable ?

[jqno]

Unfortunately EqualsVerifier doesn't work that way, and it would require major re-architecting to make it do that. Calling constructors is hard and dangerous; they can throw exceptions in unexpected ways, and they might not initialize all the fields in predictable ways. (I have some plans in that direction, but it's hard to find the time to work on them.)

But yeah, if it did call the constructor, I could possibly make that work...

[sbernard31]

Calling constructors is hard and dangerous; they can throw exceptions in unexpected ways, and they might not initialize all the fields in predictable ways.

True.
So I imagine that currently EqualsVerifier set attributes values directly using reflection API ?
The drawback is that is could create instance with attribute value not allowed by constructor.

I would never have thought that writting and testing equals/hashcode could be a so complex task until I found your library.
Anyway I learn a lot about that using it and reading the documentation, so thx a lot 🙏

(I have some plans in that direction, but it's hard to find the time to work on them.)

I'm curious to know how you plan to deal with exception ? Do you have some documentation about that ? or some thought to share ?

Maybe catching IllegalArgumentException and try with another random value ? but in some case this could be hard to find valid combination randomly? Or we need a way to express what is a valid value for each constructor argument but that sounds no so easy 🤔

[sbernard31]

I just find : https://jqno.nl/equalsverifier/manual/relaxed-equality/
And I try to use it, instead of manual test : #987 (comment), writing something like this :

@Test
public void testEquality() {
    ClassWithBigDecimalField a = new ClassWithBigDecimalField(new BigDecimal("1.0"));
    ClassWithBigDecimalField b = new ClassWithBigDecimalField(new BigDecimal("1"));
    ClassWithBigDecimalField c = new ClassWithBigDecimalField(new BigDecimal("1.00"));
    ClassWithBigDecimalField x = new ClassWithBigDecimalField(new BigDecimal("1.01"));
    EqualsVerifier.forRelaxedEqualExamples(a, b, c).andUnequalExample(x).verify();
}

but it failed with :

Precondition: two identical objects appear:
ClassWithBigDecimalField [b=1]

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
(EqualsVerifier 3.15.3, JDK 1.8.0_412 on Linux)

This is expected because my constructor create 2 identical instances and the doc clearly says :

Factory method. Asks for a list of equal, but not identical, instances of T.

But maybe it make sense to have this kind of API also for use case like this one ? 🤔

[jqno]

The plan for the constructor thing, was to make it possible for the user to choose between the reflection, or to let them provide a factory of some kind. The plans don't have a lot of detail yet :) It's a bit early to discuss them, especially since I don't know if I'll ever get to them.

The idea behind the relaxed equality is to allow classes like fractions: new Fraction(1,2) and new Fraction(2,4) should be equal, even though their individual fields aren't. I totally see why you would try to do this with your use case. Unfortunately, the way it's implemented, it won't work, because (after normalization) all the BigDecimals are equal to each other and it won't pass the precondition check (as you've demonstrated). I'd have to think about how to allow this use case...

[sbernard31]

The plan for the constructor thing, was to make it possible for the user to choose between the reflection, or to let them provide a factory of some kind. The plans don't have a lot of detail yet :) It's a bit early to discuss them, especially since I don't know if I'll ever get to them.

Let me know when your work on it, I'm curious.

I'd have to think about how to allow this use case...

👍

Thx again for your time 🙏