Stream not working in for loop

Question

BigDecimal getInterest(List<Investment> investments) {
        BigDecimal interest = BigDecimal.ZERO;
        for (Investment i: investments) {
            i.getTransactions().stream()
                    .map(Transaction::getAmount)
                    .forEach(interest::add);
        }
        return interest;
    }

The problem with this method is that it always returns zero. It looks like .forEach() is not consuming it's argument. However if I write it the way below, everything is working fine. Anyone got idea why the first method is not working?

BigDecimal getInterest(List<Investment> investments) {
        BigDecimal interest = BigDecimal.ZERO;
        for (Investment i: investments) {
            interestPaid = interest.add(i.getTransactions().stream()
                    .map(Transaction::getAmount)
                    .reduce(BigDecimal.ZERO, BigDecimal::add));
        }
        return interest;
    }

Maybe because BigDecimal is immutable, so it returns a new instance when calling add instead of updating the original one? — Koekje, Sep 07 '16 at 08:55
@fabian I missed that, thanks. Actually it's BigDecimal, but same issue. — Eran, Sep 07 '16 at 08:57
Possible duplicate of [Adding up BigDecimals using Streams](http://stackoverflow.com/questions/22635945/adding-up-bigdecimals-using-streams) — Didier L, Sep 07 '16 at 20:01

score 5 · Accepted Answer · edited May 23 '17 at 10:29

BigDecimal is immutable, so your forEach is calling add, but not doing anything with the result. reduce is the right stream operator in this case.

If you see Adding up BigDecimals using Streams. You should use .reduce(BigDecimal.ZERO, BigDecimal::add), which would make your loop body:

interest = i.getTransactions().stream()
                .map(Transaction::getAmount)
                .reduce(BigDecimal.ZERO, BigDecimal::add);

score 4 · Answer 2 · answered Sep 07 '16 at 09:01

Since BigDecimal is immutable, calling add for it won't change the value. It instead returns a new BigDecimal. forEach simply ignores any values returned.

interest will always keep it's initial value of BigDecimal.ZERO.

In contrast to that reduce combines the elements using a given BinaryOperator. BigDecimal::add is actually a short form for (a, b) -> a.add(b) and this operator will be applied to combine all stream elements.

score 1 · Answer 3 · edited May 23 '17 at 12:02

As the accepted answer explains, BigDecimal.add does not modify the instance, but returns a new value and your first variant is not using the result. As an addendum, the same would happen if you don’t use the result of reduce in you second variant, but there, you not only pass the result to an invocation of add on the previous value, you also care to assign the result of that subsequent add to your local variable.

But it’s worth noting that your mixture of loop and stream operation in inconsistent. You can express the entire operation as a single Stream operation:

BigDecimal getInterest(List<Investment> investments) {
    return investments.stream()
        .flatMap(i -> i.getTransactions().stream())
        .map(Transaction::getAmount)
        .reduce(BigDecimal.ZERO, BigDecimal::add);
}

Stream not working in for loop

3 Answers3