1

I need to find a method (using streams) to return a Map<Category,Optional<ToDo>, which help me group an ArrayList and give me a ToDo object with the highest priority of each category.

public record ToDo(String name, Category category, 
                   int priority, LocalDate date) {}
public enum Category { HOME, WORK }

An example of the input data:

List<ToDo> todo = List.of(
    new ToDo("Eat", Category.HOME, 1, LocalDate.of(2022, 8, 29)),
    new ToDo("Sleep", Category.HOME, 2, LocalDate.of(2022, 8, 30)),
    new ToDo("Learn", Category.WORK, 2, LocalDate.of(2022, 9, 3)),
    new ToDo("Work", Category.WORK, 3, LocalDate.of(2022, 10, 3))
);

And in the end, I want to have something like this as a result:

{HOME=[ToDo{Description='Eat', category=HOME, priority=1, deadline=2022-08-29},
 WORK=[ToDo{Description='Learn', category=WORK, priority=2, deadline=2022-09-03]} 

I was trying to use

.collect(Collectors.groupingBy(p -> p.getCategory()));

and

.sorted(Comparator.comparing(ToDo::getPriority)).findFirst();

But I can't do it in a single method and get Optional as a result. How can I resolve this problem?

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
bdream
  • 13
  • 4
  • 3
    Optional in a deep hierarchy is not a good idea. Instead of mapping e.g. `Category.EAT` to an `Optional.NONE`, just.. not have that mapping. – rzwitserloot Aug 29 '22 at 17:45
  • It's not my idea, it's a requirement to make it as an Optional, in detail - Map – bdream Aug 29 '22 at 17:51
  • To be clear, you need an entry in the map for every value in `Category`, even if there's no `ToDo` with a particular `Category` in the initial list? – erickson Aug 29 '22 at 18:38
  • 1
    Hint: `.sorted(Comparator.comparing(ToDo::getPriority)).findFirst()` means `.minBy(Comparator.comparing(ToDo::getPriority))` – Holger Aug 30 '22 at 08:40

2 Answers2

2

The practice of storing Optionals in a Collection is discouraged.

It might seem as a smart move at first. But in fact you're creating a Map which give you a null or potentially empty Optional, which doesn't sound very handy.

Besides that it goes against the design goal of Optional which is intended to be used as a return type. Optional is meant only for transitioning data (not storing), for that reason it was designed non-serializable, and it might cause issues.

And for every category that would be encountered in the list, there always would be a corresponding ToDo object. If your intention was to have all members of Category in the map in order to be able to safely fire an action on the Optional returned by get() via ifPresent(), then instead you can implement Null-object pattern and map your null-object ToDo to every category that wasn't present in the list via putIfAbsent().

If you want to find ToDo with the highest priority (lowest value of priority) using collector groupingBy() as you've mentioned in the question. Then you can use collector minBy() in conjunction with a collector collectingAndThen() as a downstream of groupingBy(). It would be way more efficient than combination .sorted().findFirst().

But since we need a single value mapped to each key (and not a collection of values) as @Holger has pointed out, the proper way of handling this task is by using collector toMap() instead of groupingBy() + downstream collectors. It results in less verbose and more intuitive code.

List<ToDo> todo = List.of(
    new ToDo("Eat", Category.HOME, 1, LocalDate.of(2022, 8, 29)),
    new ToDo("Sleep", Category.HOME, 2, LocalDate.of(2022, 8, 30)),
    new ToDo("Learn", Category.WORK, 2, LocalDate.of(2022, 9, 3)),
    new ToDo("Work", Category.WORK, 3, LocalDate.of(2022, 10, 3))
);

Map<Category, ToDo> highestPriorityTaskByCategory = todo.stream()
    .collect(Collectors.toMap(
        ToDo::category,
        Function.identity(),
        BinaryOperator.minBy(Comparator.comparingInt(ToDo::priority))
    ));

highestPriorityTaskByCategory.forEach((k, v) -> System.out.println(k + " -> " + v));

Output:

WORK -> ToDo[name=Learn, category=WORK, priority=2, date=2022-09-03]
HOME -> ToDo[name=Eat, category=HOME, priority=1, date=2022-08-29]
Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
  • 2
    I don’t know why, but the OP said “highest priority” but both, their posted code and expected output, indicate the *smallest* numbers for priority, i.e. “lowest value for priority”. Besides that, this is another case of “[if `groupingBy` forces you to deal with `Optional`, check whether `toMap` works](https://stackoverflow.com/a/57042622/2711488)” E.g. `.collect( Collectors.toMap(ToDo::category, Function.identity(), BinaryOperator.minBy( Comparator.comparingInt(ToDo::priority))))` (or `maxBy` instead of `minBy` whatever the OP actually wants). – Holger Aug 30 '22 at 08:47
  • @Holger That's a very good point. Updated the answer. – Alexander Ivanchenko Aug 30 '22 at 10:41
0

I assume that the DOM should be HOME, because that's the category inside your object. I also assume that the ToDoList should be an Optional<ToDo>.

The groupingBy you used will return a Map<Category, List<ToDo>>. That's the right key but the wrong value. You need to solve that by also supplying a downstream collector, that will collect ToDo elements:

Map<Category, Optional<ToDo>> result = todo.stream()
        .collect(Collectors.groupingBy(
                ToDo::getCategory,
                Collectors.minBy(Comparator.comparingInt(ToDo::getPriority))
        ));

You can improve the comparator to Comparator.comparingInt(ToDo::getPriority).thenComparing(ToDo::getDeadline) to find the entry with the earliest deadline in case some have the same lowest priority.

Note that this will only include entries for categories that are part of your input. If you need all, use an additional loop:

for (Category category : Category.values()) {
    result.computeIfAbsent(category, c -> Optional.empty());
}
Rob Spoor
  • 6,186
  • 1
  • 19
  • 20