2

How can I approach sorting with an Optional?

Both mainProducts or productSublist in the code below maybe null. The resulting list productSubList should be sorted by Date.

List<ProductSubList> productSubList = Optional.of(mainProducts)
        .map(MainProducts::getProductSubList)
        .ifPresent(list -> list.stream().sorted(Comparator.comparing(ProductSubList::getProductDate))
        .collect(Collectors.toList()));

The code shown above produces a compilation error:

Required type: ProductSubList ; Provided:void

Part Answer which seems too long, is there a way to make this shorter?

  if (Optional.of(mainProducts).map(MainProducts::getProductSubList).isPresent()) {
      productSublist = mainProducts.getProductSubList().stream()
              .sorted(Comparator.comparing(ProductSubList::getProductDate))
              .collect(Collectors.toList());
    }

Resource: How to avoid checking for null values in method chaining?

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
mattsmith5
  • 540
  • 4
  • 29
  • 67
  • hi @LouisWasserman well I need to verify ProductList and ProductSublist is not null, is there a better way? I thought thats what Optional is for https://stackoverflow.com/questions/49380052/how-to-avoid-checking-for-null-values-in-method-chaining – mattsmith5 Nov 08 '22 at 05:05
  • hi @shmosel they are both arrayLists, if mainProducts or getProductList is null, just make productSublist variable an empty list – mattsmith5 Nov 08 '22 at 05:35

2 Answers2

2

Seems you're looking for something like this:

List<ProductSubList> productSubList = Optional.ofNullable(mainProducts)
        .map(MainProducts::getProductSubList)
        .map(list -> list.stream()
                .sorted(Comparator.comparing(ProductSubList::getProductDate))
                .collect(Collectors.toList()))
        .orElse(Collections.emptyList());
shmosel
  • 49,289
  • 6
  • 73
  • 138
2

Avoid using Optional to replace Null-checks

Optional wasn't designed to perform null-checks, and there's nothing wrong implicit null-check to begin with. Yes, if there are plenty of null-checks the code can become unreadable very quickly, but it's rather a design issue than a problem with the tools offered by the language.

The best remedy for the code that suffers from null-checks is to reduce the number of nullable fields, in the first place. In some cases it's fairly easy to achieve, especially if we are talking about Collections of Arrays like in this case, simply assign an empty Collection by default. This suggestion can be found in many places, for instance, see item "Return empty collections or arrays, not nulls" of the classic book "Effective Java", by Joshua Bloch.

Regarding the usage Optional.ofNullable() here's quote from the answer by Stuart Marks, Java and OpenJDK developer:

The primary use of Optional is as follows:

Optional is intended to provide a limited mechanism for library method return types where there is a clear need to represent "no result," and where using null for that is overwhelmingly likely to cause errors.

A typical code smell is, instead of the code using method chaining to handle an Optional returned from some method, it creates an Optional from something that's nullable, in order to chain methods and avoid conditionals.

With that being said, if you would follow the advice to avoid keeping a nullable Collection and quite using Optional for null-checks the code can be written in a very simple well-readable manner.

Assuming your domain classes look like that (Lombok's @Getter annotation is used for conciseness):

@Getter
public static class ProductSubList {
    private LocalDateTime productDate;
}

@Getter
public static class MainProducts {
    private List<ProductSubList> productSubList = new ArrayList<>(); // or Collection.emptyList() if the class is mean only to carry the data and doesn't expose methods to modify the list
}

Which would give the following code:

if (mainProducts == null) return Collections.emptyList();
        
return mainProducts.getProductSubList().stream()
    .sorted(Comparator.comparing(ProductSubList::getProductDate))
    .toList();

Pretty much self-explanatory. That would be the best option if can modify the classes you're using.

If for some reason, you can't modify MainProducts and/or you've provided a simplified version and there are much more nullable moving parts, here a few more options.

Stream.ofNullable()

You can make use of the Java 9 Stream.ofNullable():

List<ProductSubList> productSubList = Stream.ofNullable(mainProducts)
    .flatMap(mainProd -> Stream.ofNullable(mainProd.getProductSubList()))
    .flatMap(Collection::stream)
    .sorted(Comparator.comparing(ProductSubList::getProductDate))
    .toList();

Note it's not a silver bullet, and I'm not saying to "plug Stream.ofNullable() everywhere". Excessive usage Stream.ofNullable() might create confusion because in it might seem there more levels of nesting than it's really is.

Stream.mapMulti()

We can reduce the number of stream operations and rearrange the code in a more readable way with Java 16 Stream.mapMulti():

List<ProductSubList> productSubList1 = Stream.ofNullable(mainProducts)
    .<ProductSubList>mapMulti((mainProd, consumer) -> {
        List<ProductSubList> prodSubLists = mainProd.getProductSubList();
        if (prodSubLists != null) prodSubLists.forEach(consumer);
    })
    .sorted(Comparator.comparing(ProductSubList::getProductDate))
    .toList();

Personally, I think the version above is good enough, but if you're unhappy with null-checks in the stream and want to see fancy one-liners, here's how it can be rewritten:

List<ProductSubList> productSubList2 = Stream.ofNullable(mainProducts)
    .<ProductSubList>mapMulti((mainProd, consumer) ->
        Objects.requireNonNullElse(
            mainProd.getProductSubList(), List.<ProductSubList>of()
        ).forEach(consumer)
    )
    .sorted(Comparator.comparing(ProductSubList::getProductDate))
    .toList();

Also see:

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
  • thanks, I have Java 8 btw, in this code, you are checking if MainProducts is null, how about productSubList, and what if it had 4-5 layers down? if (mainProducts == null) return Collections.emptyList(); return mainProducts.getProductSubList().stream() .sorted(Comparator.comparing(ProductSubList::getProductDate)) .toList(); – mattsmith5 Nov 08 '22 at 17:54
  • @mattsmith5 *"how about `productSubList`"* - have a look at the class declaration above the very first snippet, list is initialized with a default empty list. If you can make change your domain classes, then apply this fix at all levels (it's doable even if the nullable collection is coming, for instance, as API response). Eliminating nullable Collections is easier than dialing the nullable objects. – Alexander Ivanchenko Nov 08 '22 at 18:14
  • @mattsmith5 So 5 levels of nesting is not a problem if you have a leeway to refactor the code, and 4 inner levels are collections. In case if there are a lot of custom objects, contain collections that needs to be accesses, then try to consider a possibility a default initialization with some dummy data, or implementing the Null-object pattern in some cases. Anyway, eliminating `null` in **one** place is better than treating it everywhere. – Alexander Ivanchenko Nov 08 '22 at 18:53
  • @mattsmith5 Regarding the language level, in fact all snippets require **Java 16+** because of `toList()`. The first snippet can with run with **JDK 8** if you replace `toList()` with `collect(Collectors.toList())`. For that reason, I've changed the headers added (please don't get offended). – Alexander Ivanchenko Nov 08 '22 at 18:58