I have the following java method which returns the ID of a single matched movie from a list of movies, the filter criteria is the movie's attributes, all attributes provided must match:
public Optional<String> getIdFromMatchedMovie(List<Movie> movies, Map<String, Object> attributes)
{
Optional<Movie> movieOption =
movies.stream()
.filter(movie -> {
for (Map.Entry<String, Object> attr : attributes.entrySet()) {
Object returnedAttrValue = movie.getAttributes()
.get(attr.getKey());
// Attribute provided not exists in movie's attributes
// or the attributes value don't match
if (returnedAttrValue == null
|| !returnedAttrValue.equals(attr.getValue())) {
return false;
}
}
return true;
})
.findFirst();
if (movieOption.isPresent()) {
return movieOption.get().getId());
}
return Optional.empty();
}
Is there a way to rewrite this by remove the inner for loop and still leveraging the Java 8 Stream?
CodePudding user response:
Sure, you can convert the filter loop to another nested stream. Also, the isPresent()
-then-get()
is often an anti-pattern when you're using optionals. In this case, you can use Optional.map()
or even move it into the stream:
return movies.stream()
.filter(movie -> attributes.entrySet()
.stream()
.allMatch(attr -> attr.getValue().equals(
movie.getAttributes().get(attr.getKey())))
.map(Movie::getId)
.findFirst();
CodePudding user response:
You can replace the nested loop with a stream over the map entries and condition that checks the presence of the result and extracts id
property with Optional.map()
:
public Optional<String> getIdFromMatchedMovie(List<Movie> movies,
Map<String, Object> attributes) {
return movies.stream()
.filter(movie -> attributes.entrySet().stream()
.allMatch(entry -> Objects.equals(
movie.getAttributes().get(entry.getKey()),
entry.getValue()
))
.findFirst()
.map(Movie::getId);
}