Using Java 8 Streams, how to find the max for a given element in a HashMap

Question

I have a HashMap of Products. Each Product has a Price. I know how to find the Product with the max Price. But using Java 8 Streams is really puzzling me. I tried this but no luck:

public Product getMostExpensiveProduct(HashMap<Integer, Product> items) {

    Product maxPriceProduct = items.entrySet()
            .stream()
            .reduce((Product a, Product b) ->
                    a.getPrice() < b.getPrice() ? b : a);

    return maxPriceProduct;
}

Answer 1

The first problem is that since you want to find the max Product by price then it's better to use items.values() as the source for the stream as then you'll have a Stream<Product> instead of a Stream<Map.Entry<Integer, Product>>.

Second, the reduce operation doesn't have the correct types. So, to make your current code work you'll need to do something along the lines of:

Optional<Map.Entry<Integer, Product>> result = 
      items.entrySet()
           .stream()
           .reduce((Map.Entry<Integer, Product> a, Map.Entry<Integer, Product> b) ->
                        a.getValue().getPrice() < b.getValue().getPrice() ? b : a);

return result.isPresent() ? result.get().getValue() : null;

Third, this overload of the reduce operation returns an Optional<T> so the receiver type of the result set must be an Optional<T> as shown above.

Above, we're returning null in the case where there is no value present in the Optional.

A much better solution would be to make the method return a type Optional<Product>. This documents to you or your colleagues and all future users of your method that it might give an empty value as result.

This is a better alternative to returning null as it documents as well as ensures that the user of this method unwraps the return value safely.

nullity can be dangerous at times and leveraging Optional's where appropriate can take you a long way.

With all that taking into account your code becomes:

// example without returning an `Optional<T>`

public Product getMostExpensiveProduct(HashMap<Integer, Product> items) {
        Optional<Product> maxPriceProduct = 
           items.values()
                .stream()
                .reduce((Product a, Product b) ->
                        a.getPrice() < b.getPrice() ? b : a);

        return maxPriceProduct.orElse(null);
}

// example returning an Optional<T>

public Optional<Product> getMostExpensiveProduct(HashMap<Integer, Product> items) {
            Optional<Product> maxPriceProduct = 
               items.values()
                    .stream()
                    .reduce((Product a, Product b) ->
                            a.getPrice() < b.getPrice() ? b : a);

            return maxPriceProduct;
 }

Anyhow, the max method is more appropriate for this task instead of the reduce, so it can all be improved to:

Optional<Product> maxPriceProduct = 
           items.values()
                .stream()
                .max(Comparator.comparingInt(Product::getPrice));

Answer 2

Just noticed that passing a HashMap<Integer, Product> makes no sense since you are not using the Key in any way, so this could have been written to take a Collection<Product> as an argument, so it would become:

public Product getMostExpensiveProduct(Collection<Product> products) {
    return Collections.max(products, Comparator.comparingInt(Product::getPrice));
}

But then, this would be a method with a single line, so you could just call it in place where you need it instead:

Collections.max(items.values(), Comparator.comparingInt(Product::getPrice));

The downside is that this would throw a NoSuchElementException if the incoming Collection is empty.

Or if you care about all the checks of null and empty collection (and you should):

public Optional<Product> getMostExpensiveProduct(Collection<Product> products) {
    if(products == null || products.isEmpty()){
        return Optional.empty();
    }
    return Optional.of(Collections.max(
            products, Comparator.comparingInt(Product::getPrice)));
}