Assertj-core: Of extracting, tuples, comparing and BigDecimal

[sg-ad]

I've been playing around with the extraction features and contains(). Reusing the usual examples, imagine we have our Fellowship members' height:

TolkienCharacter frodo = 
    new TolkienCharacter("Frodo", 33, HOBBIT, new BigDecimal("3.750"));
TolkienCharacter sam = 
    new TolkienCharacter("Sam", 38, HOBBIT, new BigDecimal("3.50"));

and we try to extract some values and compare:

assertThat(fellowshipOfTheRing)
    .extracting("name", "height")
    .contains(
        tuple("Frodo", BigDecimal.valueOf(3.75),
        tuple("Sam", BigDecimal.valueOf(3.5));

This assertion fails because while BigDecimal.valueOf(3.75) and BigDecimal("3.750") are the same when using compareTo(), they are not considered equal when using equals() (the scale is different).

Granted this example is contrived, but it occurs more naturally when you start playing with prices that get parsed out of strings. I believe that is one of the reasons why isEqualByComparingTo() was introduced.

The only solution I found to this was to create my own Tuple comparator that handles BigDecimal objects in a very specific way. Is there any cleaner solution that I've missed? If not, could an enhancement request be considered?
Thanks!

[joel-costigliola]

Using a tuple comparator was the best option indeed.

I completely understand the pain with BigDecimal but changing the default behaviour of Tuples equals to use BigDecimal.compareTo is not an option, first it would be a breaking change, second some users might want to keep the existing behaviour.

The only solution that I see would be to register a comparison strategy for BigDecimal maybe something like usingBigDecimalCompareTo, ex :

assertThat(fellowshipOfTheRing)
    .extracting("name", "height")
    .usingBigDecimalCompareTo()
    .contains(
        tuple("Frodo", BigDecimal.valueOf(3.75),
        tuple("Sam", BigDecimal.valueOf(3.5));

This is might not be a simple change as it requires to track down where equals is used (for example in contains) and in the case of BigDecimal call compareTo instead.

Same could be said for double and float, one usually wants to set a precision to compare them.

[sg-ad]

Yes, I completely agree that changing the default behaviour of Tuples would be a bad idea. I was looking at ComparisonStrategy, trying to see how it's used and if/when one can control it. I'll confess I haven't had time to dig too far into the code, but would it make sense to allow the registration of a new comparison strategy when comparing tuples? I'm thinking something that would go along the lines of:

assertThat(fellowshipOfTheRing)
    .extracting("name", "height")
    .usingCompareToComparisonStrategy()
    .contains(
        tuple("Frodo", BigDecimal.valueOf(3.75),
        tuple("Sam", BigDecimal.valueOf(3.5));

What do you think? I haven't seen anywhere that makes the comparison strategy available/known/modifiable directly, so perhaps there are good reasons not to open this door?

[joel-costigliola]

usingCompareToComparisonStrategy might be a little bit confusing:

• would it be used for BigDecimal only or for all extracted properties ?
• if it is used for all extracted properties, what if one of the property is not Comparable ?

If you want to use a custom comparison strategy for tuples, you will need to use usingElementComparator and give it your tupleComparisonStrategy, see:

http://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html#custom-comparison-strategy

[sg-ad]

Good point. I would've proposed that this new strategy apply to all extracted properties that support Comparable and that the fallback for properties that are not Comparable would be the current behaviour.
I've been successful making my own comparator, but I did wonder if there's a broader need for it. While it does add some value to Tuple comparison, it's also getting very specific. Perhaps so much so that having it baked into assertj would benefit too few to warrant it?

[joel-costigliola]

I don't see much value to ship with different Tuple comparison strategies, I feel it's specific to each usage (but I'm happy to be proven wrong).

[sg-ad]

Yes, I think it's quite specific. Since it's never been brought up before, I'll venture it's not even a very common case. I think you can go ahead and close the issue. Thanks for the discussion, it was interesting! And also, big thanks for all the work you've been doing! :)

[joel-costigliola]

Thanks for the kind words.

[BernhardBln]

For me it's also quite often very painful.

I was wondering if usingComparatorForElementFieldsWithType couldn't get wired through into the tuple?

For example, I'd be already happy if that would work:

    ...
    .extracting("priceAmount", "priceCurrency")
            .usingComparatorForElementFieldsWithType(BigDecimal::compareTo, BigDecimal.class)
            .containsExactly(
                tuple(new BigDecimal("5.50"), "EUR"),
                tuple(new BigDecimal("2.50"), "EUR")
            );

which could be done by having containsExactly inject the registered comparators into the tuple for comparison.

Unless you don't like to mix Comparator and equals, then maybe offer a new method instead of usingComparatorForElementFieldsWithType where I can do something like this:

    ...
    .extracting("priceAmount", "priceCurrency")
            .withEqualsForElementFieldsWithType((b1, b2) -> b1.compareTo(b2) == 0, BigDecimal.class)
            .containsExactly(
                tuple(new BigDecimal("5.50"), "EUR"),
                tuple(new BigDecimal("2.50"), "EUR")
            );

I'm dealing quite often with prices, and a Tuple comparator would not really help, as the tuples always come in a different form...

[joel-costigliola]

@BernhardBln you actually can write a smart tuple comparator that compare any BigDecimal tuple element using BigDecimal.compareTo.

Having said that I will have a closer look at making usingComparatorForElementFieldsWithType usable with tuples (note though that using it alone is not enough as stated in the javadoc).

The following worked for me in AssertJ 3.8.0:

@Test
public void can_use_BigDecimal_comparator_in_tuples() {
    final List<Money> bills = Arrays.asList(new Money(new BigDecimal("5.0"), "EUR"), new Money(BigDecimal.TEN, "YEN"));
    assertThat(bills)
            .extracting("priceAmount", "priceCurrency")
            .usingElementComparator(new TupleComparator())
            .contains(tuple(new BigDecimal("5.00"), "EUR"));
}

static class Money {

    BigDecimal priceAmount;

    String priceCurrency;

    public Money(final BigDecimal priceAmount, final String currency) {
        this.priceAmount = priceAmount;
        this.priceCurrency = currency;
    }
}

// basic implementation that works but needs some love
static class TupleComparator implements Comparator<Tuple> {

    @Override
    public int compare(final Tuple t1, final Tuple t2) {
        final Object[] t1Values = t1.toArray();
        final Object[] t2Values = t2.toArray();
        for (int i = 0; i < t1Values.length; i++) {
            final Object t1Value = t1Values[i];
            final Object t2Value = t2Values[i];
            if (t1Value instanceof BigDecimal && t2Value instanceof BigDecimal) {
                final int comparison = ((BigDecimal) t1Value).compareTo((BigDecimal) t2Value);
                if (comparison != 0) {
                    return comparison;
                }
            } else if (!t1Value.equals(t2Value)) {
                return -1;
            }
        }
        return 0;
    }
}

[BernhardBln]

Hey @joel-costigliola - thanks a lot for that! I thought it would be way more complicate to write a generic one. Will use that.

The thing with usingComparatorForElementFieldsWithType - that would be super cool, as long as it doesn't break any existing code, don't know how people use that in general...

Otherwise a new switch, as suggested, would be cool :)

[BernhardBln]

This one let's you even use Strings, e.g. .contains(tuple("5.50", "EUR"))

:)

public class BigDecimalTupleComparator implements Comparator<Tuple> {

    @Override
    public int compare(final Tuple t1, final Tuple t2) {
        final Object[] t1Values = t1.toArray();
        final Object[] t2Values = t2.toArray();

        for (int i = 0; i < t1Values.length; i++) {
            final Object t1Value = t1Values[i];
            final Object t2Value = t2Values[i];

            if (t1Value instanceof BigDecimal && t2Value instanceof BigDecimal) {
                final int comparison = ((BigDecimal) t1Value).compareTo((BigDecimal) t2Value);
                if (comparison != 0) {
                    return comparison;
                }

            } else if (t1Value instanceof String && t2Value instanceof BigDecimal) {

                final int comparison =
                    (new BigDecimal((String) t1Value))
                        .compareTo((BigDecimal) t2Value);

                if (comparison != 0) {
                    return comparison;
                }

            } else if (t1Value instanceof BigDecimal && t2Value instanceof String) {

                final int comparison =
                    new BigDecimal((String) t2Value)
                        .compareTo((BigDecimal) t1Value);

                if (comparison != 0) {
                    return comparison;
                }

            } else if (!t1Value.equals(t2Value)) {
                return -1;
            }
        }

        return 0;
    }
}

[LostOblivion]

I spent hours trying to understand why usingComparatorForType
or even usingComparatorForElementFieldsWithType does not work with tuples (in my case for comparing doubles). As it is now, it is very easy to mistake this as a bug in assertj. Is it non-trivial to add support for tuples?

I also ended up writing a custom tuple comparator, but it feels very clunky, and should be part of assertj itself.

[joel-costigliola]

@LostOblivion can you give a try to new recursive API ? As you can ignore overriden equals method (Tuple in this case), it might just solve this issue.

https://assertj.github.io/doc/#assertj-core-recursive-comparison-ignoring-equals

[LostOblivion]

This is my code right now, if you want to see. consumptions is a list of elements of a given class. Intuitively, usingComparatorForType should work for this too.

        assertThat(consumptions)
                .extracting("consumer.name", "service.name", "shareOfUsage")
                .usingComparatorForType(new DoubleComparator(.00001), Double.class)
                .contains(
                        tuple("BigConsumer", "ServiceOne", .34394),
                        tuple("BigConsumer", "ServiceTwo", .32668),
                        tuple("SmallConsumer", "ServiceThree", .01725),
                        tuple("SmallConsumer", "ServiceFour", .13190),
                        tuple("SmallConsumer", "ServiceFive", .18020)
                );

I'll look at your example in the mean while, but at first sight, does this work for collections though?

[LostOblivion]

@joel-costigliola , I couldn't get it to work with recursive comparison as the assertThat for collections does not have a method usingRecursiveComparison nor isEqualToComparingFieldByFieldRecursively.

When looking at the code above, there really is no intuitive reason why it shouldn't work. extractors and tuples are one of the main reasons we use assertj in our project, but right now its pretty useless for doubles.

[joel-costigliola]

I agree it is not intuitive, the good news is recursive comparison collection support is coming in the next release (as a Beta though).

I haven't tried but you might get to work with:

// forces to consider List as an Object so that you can call usingRecursiveComparison()
assertThatObject(list).usingRecursiveComparison()
                      .ignoringOverriddenEqualsForTypes(Tuple.class)
                      .withComparatorForType(new DoubleComparator(.00001), Double.class)
                      .isEqualTo(expectedList);

The limitation is that you can only use isEqualTo as collections assertions are not supported yet.

Again, the thing is Tuple has already an equals method so you have to provide a Comparator for it that deals with Double as you want.