CODEWITHSUNDEEP
javajava-8java-stream
Alain Duguine
Alain Duguine

Reputation: 475

Best practice to map and filter a collection of object in java using streams

I have a List of custom entities from which i want to valuate a field using a method, and that i want to filter next. I'm fairly new to java streams, and i don't know if it's better to use map and filters, or to use a more traditional forEach. Here is my first attempt:

public List<Restaurant> findRestaurantsWithin(Double latitude, Double longitude, Integer radius) {
    log.info("Searching restaurants {} km to point {} lat., {} long.", radius, latitude, longitude);

    List<Restaurant> restaurants = this.restaurantRepository.findAll();

    restaurants.forEach(restaurant ->
        {
            if (restaurant.getLatitude() != null) {
                restaurant.setDistance(
                        this.getDistance(Double.parseDouble(restaurant.getLatitude()),
                                Double.parseDouble(restaurant.getLongitude()), latitude, longitude)
                );
            }
        }
    );

    return restaurants.stream()
            .filter(restaurant -> restaurant.getDistance() <= radius)
            .sorted(Comparator.comparing(Restaurant::getDistance))
            .skip(size * page - 1)
            .limit(size)
            .collect(Collectors.toList());

And here is the second :

return this.restaurantRepository.findAll().stream()
                .filter(restaurant ->
                {
                    if (restaurant.getLatitude() != null) {
                        Double distance = this.getDistance(Double.parseDouble(restaurant.getLatitude()), Double.parseDouble(restaurant.getLongitude()), latitude, longitude);
                        if (distance <= radius) {
                            restaurant.setDistance(distance);
                            return true;
                        }
                    }
                    return false;
                })
                .sorted(Comparator.comparing(Restaurant::getDistance))
                .skip(size * page - 1)
                .limit(size)
                .collect(Collectors.toList());

On the second one i should probably use a .map first, but i'm not sure that there will be a difference in performance. Is there a better practice or more elegant way to get that ? Thanks !

Upvotes: 0

Views: 743

Answers (1)

Michael
Michael

Reputation: 44200

I'd say that Restaunt::setDistance is the big code smell here. Each restaurant is capable of holding property which says how far away it is from some other arbitrary point. We don't know where or what that point is, or when it was set. It's not a true property of a restaurant, it's just a hack.

Side effects and functional programming in general do not really mix well either, which is another reason both of your examples might seem clunky to you.

Here's how I'd do it, with a general purpose Pair class (there are many but the exact implementation shouldn't matter. Java FX was removed in I think Java 11, but if you're using a version of Java before that, javafx.util.Pair requires no dependencies)

return restaurantRepository.findAll().stream()
    .filter(restaurant -> restaurant.getLatitude() != null)
    .map(restaurant -> new Pair<>(restaurant, this.getDistance(/*blah blah*/)))
    .filter(resAndDistance -> resAndDistance.getValue() <= radius)       
    .sorted(Comparator.comparing(Pair::getValue))
    .skip(size * page - 1)
    .limit(size)
    .collect(Collectors.toList());

Upvotes: 3

Related Questions