Krdytkyu

What I'm trying to do is to filter the list, then map it and use orElse if null and then collect it back to the list. Now I can achieve it this way:

return users

    .stream()

    .filter(user -> id.equals(user.getId()))

    .map(

        user -> {

            if(user.getData() != null) {

                return user.getData();

            }

            return Collections.emptyMap();

        }

    )

    .collect(Collectors.toList())

;

But the question is: how can I make this structure better and why can I not use orElse in this case?

It might be more readable with ternary conditional operator:

return users.stream()

    .filter(user -> id.equals(user.getId()))

    .map(

        user -> (user.getData() != null) 

        ? user.getData() 

        : emptyMap()

    )

    .collect(Collectors.toList())

;

In order to use orElse you'll have to create an Optional that wraps user.getData(). I'm not sure that's a good idea.

If you insist on using orElse (or even better, orElseGet, to avoid evaluating emptyMap() when it's not required), it can look like this:

return users.stream()

    .filter(user -> id.equals(user.getId()))

    .map(

        user -> Optional.ofNullable(

            user.getData()

        ).orElseGet(

            () -> emptyMap()

        )

    )

    .collect(Collectors.toList())

;

As I pointed out in the comments as well and I highly doubt that you might just be looking for the following

users

    .stream()

    .filter(

        user -> id.equals(user.getId()) 

        && (user.getData() != null)

    )

    .map(User::getData)

    .collect(Collectors.toList())

;

But then the question isn't clear enough to say what is the eventual return type of your statement is or what is the emptyMap used in your code! Hence I highly doubt, if you even need an Optional API in first place for this operation.

Note: The above-stated solution does assume that emptyMap is Collections.emptyMap which I am not sure why would one want to collect in a data structure which is denoted as List<Map<K,V>>.

What about:

return users

    .stream()

    .filter(user -> id.equals(user.getId()))

    .map(user -> user.getData())

    .filter(Objects::nonNull)

    .collect(Collectors.toList())

;

Excerpt from Objects::nonNull:

API Note:
This method exists to be used as a Predicate, filter(Objects::nonNull)

Use Objects::requireNonNullElse!

I would advise of of two things to make the code more readable. I would not, however, artificially introduce an Optional.

First option: Objects::requireNonNullElse in a separate method

List<Map<?, ?> bar() {

    //...



    return users

        .stream()

        .filter(user -> id.equals(user.getId()))

        .map(User::getData)

        .map(Foo::nullSafeMap)

        .collect(Collectors.toList())

    ;

}



private static Map<?, ?> nullSafeMap(final Map<?, ?> map) {

    return Objects.requireNonNullElse(map, Collections.emptyMap());

}

Here, you would use Objects::requireNonNullElse, which returns the object passed in the first parameter if it is not null, and the object passed as the second parameter if the first parameter is null. Having a separate method allows for a method reference to be passed to Stream::map, but requires you to first map the User instances to their data Map.

Second option: Inline Objects::requireNonNullElse

List<Map<?, ?> bar() {

    //...



    return users

        .stream()

        .filter(user -> id.equals(user.getId()))

        .map(User::getData)

        .map(map -> Objects.requireNonNullElse(map, Collections.emptyMap()))

        .collect(Collectors.toList())

    ;

}

If you do not want a separate method to do just this single task, you can inline the method and optionally even remove the first mapping in favor of .map(user -> Objects.requireNonNullElse(user.getData(), Collections.emptyMap())), but I would advise against this. Don't be afraid to have multiple calls to Stream::map if it makes the code more readable.

Conclusion

I would prefer the first option as it makes the code very readable: You know that you map the User instances to the data, then you make that data null safe.

The second option is alright, but suffers from a very long line that might be confusing on the first glance. It is much better than having a multi-line lambda though. I would avoid multi-line lambdas at all costs and always extract their contents into a separate method.

One thing you might be able to improve upon is the method name nullSafeMap, as to avoid confusion between Stream::map and java.util.Map.

Note that you don't need to use Objects::requireNonNullElseGet since Collections::emptyMap is a lightweight method that only casts and returns a constant:

public static final <K,V> Map<K,V> emptyMap() {

    return (Map<K,V>) EMPTY_MAP;

}

Objects::requireNonNullElseGet is made for default objects whose retrieval or creation is heavyweight.