Convert null checks to Optional

Question

I don't understand how to change these null checks with Optional in a functional way:

private boolean findProduct(String prodName) {
    for(OrderItem item : orderItems) {
        if(item != null) {
            Product p=item.getProduct();
            if(p != null) {
                String name = p.getProductName();
                if(name != null) {
                    if(name.equals(prodName)) return true;
                }
            }
        }
    }
    return false;       
}

Ruslan · Accepted Answer · 2019-03-17T23:43:46.553

4

Use Optional.ofNullable and Optional.map:

for(OrderItem item : orderItems) {
    Optional<String> prod = Optional.ofNullable(item)
            .map(OrderItem::getProduct)
            .map(Product::getProductName)
            .filter(s -> s.equals(prodName));

    if (prod.isPresent()) {
        return true;
    }
}
return false;

See javadoc for Optional.map:

If a value is present, apply the provided mapping function to it, and if the result is non-null, return an Optional describing the result. Otherwise return an empty Optional.

edited Mar 17 '19 at 23:43

answered Mar 17 '19 at 23:34

Ruslan

6,090
1
21
36

@LppEdd Oops, I though that was a `java.util.Stream` object :P – Cardinal System Mar 17 '19 at 23:43
the if clause is unnecessary. Simply do `return prod.isPresent()`. – akortex Mar 18 '19 at 00:02
Also this can simplified even more. – akortex Mar 18 '19 at 00:03
1

@Aris_Kortex You can't do `return prod.isPresent()` because you need to check every item inside the loop – Ruslan Mar 18 '19 at 00:09

akortex · Answer 2 · 2019-03-18T13:55:36.003

3

You can also use the Stream API to perform what you want

public boolean findProductOptional(String productName) {
   return orderItems
     .stream()
     .filter(Objects::nonNull)
     .map(OrderItem::getProduct)
     .filter(Objects::nonNull)
     .anyMatch(product -> productName.equals(product.getProductName()));
}

Simply stream the list of order items, map to a product and check whether a product with the given name exists.

edited Mar 18 '19 at 13:55

answered Mar 18 '19 at 00:04

akortex

5,067
2
25
57

@Ruslan granted that you can get a NPE if the `prodcuctName` is null but this also the case for your code too (where you never check whether it's null or not). – akortex Mar 18 '19 at 00:30
I use `Optional` to avoid null checks. `Optional` is invented exactly for this. Just try it.. there is no NPE in my case – Ruslan Mar 18 '19 at 00:34
1

@Ruslan no, optional was not invented for this. All these “*replace simple null checks with complicated optional use*” refactorings are countering the original intent of this API. Anyway, this stream solution is much simpler and just changing the last part to `.anyMatch(product -> productName.equals(product.getProductName()))` would allow it to deal with `null` product name property, not that having such a bunch of nullable properties was a good coding style… – Holger Mar 18 '19 at 13:49
@Holger, also another good point to made here, is the fact that this way you avoid dealing with the overhead or wrapping and creating a new `Optional` object for each element in the list. People are most of the time oblivious of the overhead that this process may entail. `Optional` is fine for other usages but not this case. – akortex Mar 18 '19 at 13:54
@Holger I agree that having a bunch of nullable is not a good style.. But for example this [`oracle article`](https://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html) says that you can protect your code against unintended null pointer exceptions with `Optional` and I don't see any *complicated optional use* here. – Ruslan Mar 18 '19 at 14:17
@Aris_Kortex `Optional` is just a simple container. I'm sure there are some cases of `Optional` abuse as well as `Stream`, but this example hardly ever entail some overhead. More about [`Optional overhead`](https://stackoverflow.com/questions/34696884/performance-of-java-optional) – Ruslan Mar 18 '19 at 14:24
@Ruslan that article is about methods returning `Optional` in the first place. – Holger Mar 18 '19 at 14:59

Luca Cucchetti · Answer 3 · 2019-03-19T03:42:06.597

Sort of a like a combination of the answers above, you can use both streams and optional (plus it’s a one-liner).

private boolean findProduct(String prodName) {
        return orderItems.stream()
                .map(Optional::ofNullable)
                .anyMatch(o -> o
                        .map(OrderItem::getProduct)
                        .map(Product::getProductName)
                        .map(s -> s.equals(prodName))
                        .isPresent());
}

Convert null checks to Optional

3 Answers3