Reputation: 764
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 cannot I use orElse
in this case?
Upvotes: 18
Views: 21415
Reputation: 3557
Objects::requireNonNullElse
!I would advise of of two things to make the code more readable. I would not, however, artificially introduce an Optional
.
Objects::requireNonNullElse
in a separate methodList<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
.
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.
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.
Upvotes: 1
Reputation: 1746
If you already have Apache Collections 4 as dependency:
return users
.stream()
.filter(user -> id.equals(user.getId()))
.map(User::getData)
.map(MapUtils::emptyIfNull)
.collect(Collectors.toList())
;
If you don't use Apache Collections just define a helper method:
public static <K,V> Map<K,V> emptyIfNull(Map<K,V> map) {
return map == null ? Collections.<K,V>emptyMap() : map;
}
Upvotes: 0
Reputation: 31878
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>>
.
Upvotes: 4
Reputation: 4363
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
If user.getData()
returns null
, it should be wrapped to an Optional
to call orElse
.
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.
Upvotes: 1
Reputation: 393781
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())
;
Upvotes: 15