0

I have a method that takes an object having an ArrayList inside:

private List<String> getWatchlistDetail(MatchData k) {
        if(k == null)
            return new ArrayList<>();
        return
                k.getWatchListDetail()
                        .stream()
                        .collect(Collectors.toList());
    }

MatchData k can be null, and sometimes k.getWatchListDetail() also could be null.

I need to check both condition. First, if it can throw NPE.

Above implementation does it, but I am trying to use perhaps Optional.ofNullable() or streams with chaining, so I can do it in one line of chaining.

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46

1 Answers1

1

MatchData k is null, sometimes k.getWatchListDetail() is also null so I need to check both

... so I can do it in one line of chaining.

Sure, you can check both in a single stream statement.

private List<String> getWatchlistDetail(MatchData k) {
    
    return Stream.ofNullable(k)
        .map(MatchData::getWatchListDetail)
        .filter(Objects::nonNull)
        .flatMap(List::stream)
        .collect(Collectors.toList());
}

But, keep in mind:

  • It's not a remedy, but an attempt to hide a design flaw.
  • It has a cost of generating a new completely identical collection. Which is not good at all.

Now compare it with the method below, which neither hides null-checks, no abuses streams and optional:

private List<String> getWatchlistDetail(MatchData k) {
    
    return k != null && k.getWatchListDetail() != null ?
           k.getWatchListDetail() : Collections.emptyList();
}

Functional programming was introduced in Java to reduce complexity of code, not the opposite, using it only for the purpose of hiding null-checks would not bay you anything.

By itself, null-check are not a problem, but only an indicator of a problem. And to address it, need to refine the design of your application.

use perhaps Optional.ofNullable

In regard to the question whether it's possible to utilize optional in order to avoid conditional logic, take a look at this answer by @StuartMarks (JDK 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.

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
  • good explanation on the use of Optionals. So my current one is some what right in term of checking for null first before returning anything right? Thanks alot! – نور شحلام ابن مخسين May 08 '22 at 14:34
  • @نورشحلامابنمخسين Yes, you get it correctly, there's nothing wrong with explicit null-check because it's very obvious what the code does. But you don't need to generate a new list, it increases memory consumption and make the code complicated. Just use the existing one. I understand that you're forced to use code that produces null values, and maybe you can't change it. To begin with, you might isolate all null check in one place, so that when you need to use your object there will be no need to guard yourself from null. And gradually move towards replacing this legacy code. – Alexander Ivanchenko May 08 '22 at 15:40
  • @نورشحلامابنمخسين And if you're in control of the code that gives you nullable values - refactor it to return `Optional` objects and empty collections instead of null. – Alexander Ivanchenko May 08 '22 at 15:44
  • 1
    At least, using optional, e.g. `return Optional.ofNullable(k) .map(MatchData::getWatchListDetail) .orElse(Collections.emptyList());` would be a thousand times better than this stream operation: `Stream.ofNullable(k) .map(MatchData::getWatchListDetail) .filter(Objects::nonNull) .flatMap(List::stream) .collect(Collectors.toList())` But I’d still prefer explicit `null` checks. I’d eves use a local variable to avoid calling `getWatchListDetail()` twice. Squeezing everything into a single statement is not a useful goal… – Holger May 11 '22 at 07:16