Java-Stream - Abnormal behaviour while using Collector reducing()

Question

I'm trying to solve the problem described in the following question using collector reducing():

Creating Map of Maps using a custom Object as a reduction type in Java 8

We need to obtain a Map<String,Map<String,MySumObject>> as a result by adding up BigDecimal amounts of loan objects represented by MyObject while grouping the data by loanType then loanCurrency.

Dummy classes:

class MyObject {
    String loanType;
    String loanCurrency;
    BigDecimal amountPaid;
    BigDecimal amountRemaining;
}

class MySumObject {
    BigDecimal paidSum;
    BigDecimal remainingSum;
}

Based on Alexander Ivanchenko's solution (see the link to original question) using

Collector.of(
    MySumObject::new,
    MySumObject::addLoan,
    MySumObject::merge
)

Firstly, I've changed it in the following way:

list.stream().collect(groupingBy(MyObject::getLoanType,
    groupingBy(MyObject::getLoanCurrency,
        Collector.of(
            MySumObject::new,
            (mySumObject, myObject) -> {
                mySumObject.setPaidSum(mySumObject.getPaidSum().add(myObject.getAmountPaid()));
                mySumObject.setRemainingSum(mySumObject.getRemainingSum().add(myObject.getAmountRemaining()));
            },
            (mySumObject1, mySumObject2) -> {
                mySumObject1.setPaidSum(mySumObject1.getPaidSum().add(mySumObject2.getPaidSum()));
                mySumObject1.setRemainingSum(mySumObject1.getRemainingSum().add(mySumObject2.getRemainingSum()));
                return mySumObject1;
            })
        )
    ));

Then I was trying to make it working using collector reducing().

But it appears that it adds up everything together regardless of groupingBy(). Not sure which part is wrong:

List<MyObject> list = List.of(
    new MyObject("Type1", "Currency1", BigDecimal.valueOf(10), BigDecimal.valueOf(100)),
    new MyObject("Type1", "Currency1", BigDecimal.valueOf(10), BigDecimal.valueOf(100)),
    new MyObject("Type2", "Currency2", BigDecimal.valueOf(20), BigDecimal.valueOf(200)),
    new MyObject("Type3", "Currency3", BigDecimal.valueOf(30), BigDecimal.valueOf(300)),
    new MyObject("Type4", "Currency4", BigDecimal.valueOf(40), BigDecimal.valueOf(400))
);

list.stream().collect(groupingBy(MyObject::getLoanType,
    groupingBy(MyObject::getLoanCurrency,
        reducing(new MySumObject(BigDecimal.ZERO,BigDecimal.ZERO),
            (myObject) -> new MySumObject(
                myObject.getAmountPaid(),
                myObject.getAmountRemaining()
            ),
            (mySumObject1, mySumObject2) -> {
                mySumObject1.setPaidSum(mySumObject1.getPaidSum().add(mySumObject2.getPaidSum()));
                mySumObject1.setRemainingSum(mySumObject1.getRemainingSum().add(mySumObject2.getRemainingSum()));
                return mySumObject1;
            })
        )
    ));

Here is the output it produces. All the values are added together, which is incorrect.

Type2={Currency2=MySumObject(paidSum=110, remainingSum=1100)}
Type3={Currency3=MySumObject(paidSum=110, remainingSum=1100)}
Type4={Currency4=MySumObject(paidSum=110, remainingSum=1100)}
Type1={Currency1=MySumObject(paidSum=110, remainingSum=1100)}

Answer 1

Operation reduce() is meant folding the stream by performing reduction on immutable objects (the same holds true for collector reducing()). On the other hand, collect() operation is meant for performing mutable reduction.

The observed caused by mixing the two approaches together - you're mutating the identity instead of returning a new object. That not how reduce should function.

As a result, all the entries of the nested Map would hold the reference to the same value. That would an instance of MySumObject provided as the identity of reduction. Reminder: reducing() as its first argument identity of type U (not a Supplier), hence only a single identity object would be created (you can print a message from the constructor to make sure that it would be fired only once).

In order to apply collector reducing() for solving this problem you need to change the reduction logic instead of mutating the identity, every step of reduction should produce a new MySumObject.

For that, we need to change the third argument of reducing(), which is BinaryOperator<U> used for combining the two instances of MySumObject.

And it would be more handy to introduce a mapper method in the MyObject class.

Sidenote: try to use more meaningful names. It makes it easier to work with the code. For instance, Loan instead of MyObject, LoanSum instead of MySumObject.

That's how it might be implemented:

Map<String, Map<String, MySumObject>> result = list1.stream().collect(
    Collectors.groupingBy(
        MyObject::getLoanType,
        Collectors.groupingBy(
            MyObject::getLoanCurrency,
            Collectors.reducing(
                new MySumObject(),
                MyObject::toMySum,
                MySumObject::merge
            ))
    ));

MySumObject class with a merge() method which returns a new object:

public static class MySumObject {
    private BigDecimal paidSum = BigDecimal.ZERO;
    private BigDecimal remainingSum = BigDecimal.ZERO;
    
    public void addLoan(MyObject loan) {
        paidSum = paidSum.add(loan.getAmountPaid());
        remainingSum = remainingSum.add(loan.getAmountRemaining());
    }
    
    public MySumObject merge(MySumObject other) {
        BigDecimal newPaidSum = paidSum.add(other.getPaidSum());
        BigDecimal newRemainingSum = remainingSum.add(other.getRemainingSum());
        return new MySumObject(newPaidSum, newRemainingSum);
    }
    
    // AllArgs & NoArgs constructors, getters, etc.
}

MyObject class with mapper method toMySum():

public static class MyObject {
    private String loanType;
    private String loanCurrency;
    private BigDecimal amountPaid;
    private BigDecimal amountRemaining;
    
    public MySumObject toMySum() {
        return new MySumObject(this.amountPaid, this.amountRemaining);
    }
    
    // constructor, getters, etc.
}

A link to Online Demo