Correct syntax for Collectors.reducing

Question

I have an Order class and a LineItem class like below:

@AllArgsConstructor
@Getter
@ToString
static class Order {
    long orderId;
    List<LineItem> lineItems;
}

@AllArgsConstructor
@Getter
@ToString
static class LineItem {
    String name;
    BigDecimal price;
}

and a list of orders, from which I want to get a map Map<String,BigDecimal> totalByItem where key is name of LineItem and value total price from all orders in the list. For this I want to use Collectors.groupingBy in combination with Collectors.reducing but struggling with the correct syntax. Can someone help?

List<Order> orders = List.of(new Order(1L, List.of(new LineItem("Item-A", BigDecimal.valueOf(1)),
                                                   new LineItem("Item-B", BigDecimal.valueOf(2)),
                                                   new LineItem("Item-C", BigDecimal.valueOf(3)))),
                             new Order(2L, List.of(new LineItem("Item-A", BigDecimal.valueOf(1)),
                                                   new LineItem("Item-D", BigDecimal.valueOf(4)),
                                                   new LineItem("Item-E", BigDecimal.valueOf(5)))),
                             new Order(3L, List.of(new LineItem("Item-B", BigDecimal.valueOf(2)),
                                                   new LineItem("Item-C", BigDecimal.valueOf(3)),
                                                   new LineItem("Item-D", BigDecimal.valueOf(4)))));

what to put where there are ??? now?

Map<String,BigDecimal> totalByItem =
         orders.stream()
                 .flatMap(order -> order.getLineItems().stream())
                 .collect(Collectors.groupingBy(LineItem::getName,
                          lineItem -> Collectors.reducing(BigDecimal.ZERO,(a,b) -> ???)));

Why did you pass a lambda to the second parameter of `groupingBy`? That's supposed to be a `Collector`. — Sweeper, Mar 21 '22 at 14:23
@Sweeper I have also tried without a lambda, `Collectors.reducing(BigDecimal.ZERO,BigDecimal::add)` but getting an error `Type parameter T has incompatible upper bounds: BigDecimal and LineItem` I thought to extract the price, I need to use a lambda!? — bbKing, Mar 21 '22 at 14:27
You are reducing a bunch of `LineItem`s to a `BigDecimal`, so you should use [the 3-parameter overload](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/stream/Collectors.html#reducing(U,java.util.function.Function,java.util.function.BinaryOperator)) instead. — Sweeper, Mar 21 '22 at 14:31

WJS · Answer 1 · 2022-03-21T14:37:42.507

You had it almost perfect. You need to call mapping to pull the price from LineItem. Then you can do the reducing operation as shown below.

Map<String, BigDecimal> totalByItem = orders.stream()
        .flatMap(order -> order.getLineItems().stream())
        .collect(Collectors.groupingBy(LineItem::getName,
                Collectors.mapping(LineItem::getPrice,
                        Collectors.reducing(
                        BigDecimal.ZERO, BigDecimal::add))));

Note that you could have done the reducing operation as follows:

Collectors.reducing(BigDecimal.ZERO, (a,b)->a.add(b)))));

With your current data, here is what prints totalByItem.entrySet().forEach(System.out::println);

Item-A=2
Item-E=5
Item-D=8
Item-C=6
Item-B=4

Sweeper · Accepted Answer · 2022-03-22T08:58:22.373

groupingBy takes a Collector as its second argument, so you should not pass the lambda lineItem -> ..., and instead pass the Collector.reducing(...) directly.

Also, since you are reducing a bunch of LineItems to one BigDecimal, you should use the three-parameter overload of reducing, with a mapper

public static <T, U> Collector<T,?,U> reducing(
    U identity,
    Function<? super T,? extends U> mapper,
    BinaryOperator<U> op)

The mapper is where you specify how a LineItem into a BigDecimal. You probably confused this with the second parameter of groupingBy.

So to summarise:

Map<String,BigDecimal> totalByItem =
    orders.stream()
        .flatMap(order -> order.getLineItems().stream())
        .collect(
            Collectors.groupingBy(
                LineItem::getName,
                Collectors.reducing(
                    BigDecimal.ZERO,
                    LineItem::getPrice, // <----
                    BigDecimal::add
                )
            )
        );

As Holger commented, the entire groupingBy collector can also be replaced with a toMap collector, without using reducing at all.

.collect(
    Collectors.toMap(
        LineItem::getName, // key of the map
        LineItem::getPrice, // value of the map
        BigDecimal::add // what to do with the values when the keys duplicate
    )
);

As (almost) always when combining `groupingBy` with `reducing`, just using `toMap` will be simpler and potentially more efficient: `Collectors.toMap(LineItem::getName, LineItem::getPrice, BigDecimal::add)` — Holger, Mar 22 '22 at 08:52
@Holger Thank you! It always amazes me how one can reach a much better solution by just framing the question in a different way/thinking about it from a different perspective, which I admittedly often fail to do. — Sweeper, Mar 22 '22 at 09:00

Correct syntax for Collectors.reducing

2 Answers2