CODEWITHSUNDEEP
arraylistjava-8hashmapjava-stream
josephine preethi
josephine preethi

Reputation: 33

Java 8 Stream API - convert for loop over map & list iterator inside it

In the below code, I am trying to calculate the total price of a basket, where basket is a HashMap containing the products as key and the quantity as value. Promotions are available as a list of Promotion.

I am looping over every map entry and for each of them iterating the promotions. If the promotion matches, I am taking the promotion price (promotion.computeDiscountedPrice()) and removing the promotion from the list (Because a promotion is applicable only to a product & product is unique in the list)

If there is no promotion, we execute block.

if (!offerApplied) { /* .... */ }

Can you please help me in doing this same operation using JAVA 8 stream api? 

BigDecimal basketPrice = new BigDecimal("0.0");
Map<String, Integer> basket = buildBasket(input);
List<Promotion> promotions = getOffersApplicable(basket);

for (Map.Entry<String, Integer> entry : trolley.entrySet()) {
    boolean offerApplied = false;
    Iterator<Promotion> promotionIterator = promotions.iterator();
    while (promotionIterator.hasNext()) {
    Promotion promotion = promotionIterator.next();
    if (entry.getKey().equalsIgnoreCase(offer.getProduct().getProductName())) {
        basketPrice = basketPrice.add(promotion.computeDiscountedPrice());
        offerApplied = true;
        promotionIterator.remove();
        break;
    }
    if (!offerApplied) {
        basketPrice = basketPrice.add(Product.valueOf(entry.getKey()).getPrice()
                        .multiply(new BigDecimal(entry.getValue())));
    }
}
return basketPrice;

Upvotes: 1

Views: 1389

Answers (1)

Jose da Silva Gomes
Jose da Silva Gomes

Reputation: 3974

The simplest and cleaner solution, with a better performance than having to iterate the entire promotions list, is to start by creating a map of promotions identified by the product id (in lower case or upper case [assuming no case collision occurs by the use of equalsIgnoreCase(..)]).

Map<String, Promotion> promotionByProduct = promotions.stream()
        .collect(Collectors.toMap(prom -> prom.getProduct()
                .getProductName().toLowerCase(), Function.identity()));

This will avoid the need to iterate over the entire array when searching for promotions, it also avoids deleting items from it, which in case of being an ArrayList would need to shift to left the remaining elements each time the remove is used.

BigDecimal basketPrice = basket.keySet().stream()
        .map(name -> Optional.ofNullable(promotionByProduct.get(name.toLowerCase()))
                .map(Promotion::computeDiscountedPrice) // promotion exists
                .orElseGet(() -> Product.valueOf(name).getPrice()) // no promotion
                .multiply(BigDecimal.valueOf(basket.get(name))))
        .reduce(BigDecimal.ZERO, BigDecimal::add);

It iterates for each product name in the basket, then checks if a promotion exists, it uses the computeDiscountedPrice method, otherwise it looks the product with Product.valueOf(..) and gets the price, after that it mutiplies this value by the quantity of products in the basket and finally the results are reduced (all values of the basket are added) with the BigDecimal.add() method.

Important thing to note, is that in your code, you don't multiply by the quantity the result of promotion.computeDiscountedPrice() (this code above does), i'm not sure if that is a type in your code, or that's the way it should behave.

If case it is in fact the way it should behave (you don't want to multiply quantity by promotion.computeDiscountedPrice()) the code would be:

BigDecimal basketPrice = basket.keySet().stream()
        .map(name -> Optional.ofNullable(promotionByProduct.get(name.toLowerCase()))
                .map(Promotion::computeDiscountedPrice)
                .orElseGet(() -> Product.valueOf(name).getPrice()
                        .multiply(BigDecimal.valueOf(basket.get(name)))))
        .reduce(BigDecimal.ZERO, BigDecimal::add);

Here the only value multiplied by quantity would be the product price obtained with Product.valueOf(name).getPrice().

Finally another option, all in one line and not using the map (iterating over the promotions) using the first approach (multipling by quantity in the end):

BigDecimal basketPrice = basket.keySet().stream()
        .map(name -> promotions.stream()
                .filter(prom -> name.equalsIgnoreCase(prom.getProduct().getProductName()))
                .findFirst().map(Promotion::computeDiscountedPrice) // promotion exists
                .orElseGet(() -> Product.valueOf(name).getPrice()) // no promotion
                .multiply(BigDecimal.valueOf(basket.get(name))))
        .reduce(BigDecimal.ZERO, BigDecimal::add);

Upvotes: 1

Related Questions