Cfrtjryk

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>>.

How can I make this structure better

Method 1:

return users.stream()

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

    .map(

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

        ? user.getData() 

        : emptyMap()

    )

    .collect(Collectors.toList())

;

Method 2:

Make your getData return an Optional: user -> user.getData().orElse(emptyMap())

Method 3:

As @Eran said: Optional.ofNullable then use orElse(emptyMap()) like above: user -> Optional.ofNullable(user.getData()).orElse(emptyMap())

Why I cannot use orElse in this case?

Not sure what orElse you mean

1. If user.getData() returns null, it should be wrapped to an Optional to call orElse.




2. The stream's findAny().orElse operates on the stream's result itself. But what you need here is to check if user.getData() exists. So you can not use stream's result's orElse directly.

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.