Java 8 - Browse list, change found case, if not, add

Question

In a shopping cart app, I have inside the class CarrinhoCompras the following attribute:

List<Item> itens = new ArrayList<>();

And a method to add item, which receives a product, its unit value and quantity. What the function does is to search in the ArrayList of items if the product passed as parameter is already in the cart. If the item is, it changes its value and if it is not adding a new item.

I’m looking to settle purely on the appeal of streams, but I think I’m doing too much processing for that. The method has the following code:

public void adicionarItem(Produto produto, BigDecimal valorUnitario, int quantidade) {

    itens.stream().filter(item -> item.getProduto().equals(produto)).findAny()
    .map(item -> {
        item.setQuantidade(item.getQuantidade() + quantidade);
        item.setValorUnitario(valorUnitario);
        return item;
    }).orElseGet(() -> {

        itens.add(new Item(produto, valorUnitario, quantidade));
        return null;

    });
}

Does anyone know a way to do this check without having to go through so much processing?

Answer 1

If you’re worried about processing, maybe shouldn’t wear streams, since they have their cost and for that reason are slower than a loop traditional.

Another point is that maybe you’re complicating the code for nothing. You don’t always need to streams. Your code could go like this:

public void adicionarItem(Produto produto, BigDecimal valorUnitario, int quantidade) {
    for (Item item : itens) {
        if (item.getProduto().equals(produto)) {
            item.setQuantidade(item.getQuantidade() + quantidade);
            item.setValorUnitario(valorUnitario);
            return; // achou o item, pode sair do método
        }
    }
    // se chegou aqui é porque não encontrou um item com o mesmo produto
    itens.add(new Item(produto, valorUnitario, quantidade));
}

---

But if you really want to use it stream, maybe use map is not the best choice, as this method serves to transform one value into another (for example, map(item -> item.getProduto()) causes the original value (item) to be "transformed" into its respective product).

Use map to do something with the item and then return it - although it works - is a "crooked use" of this method. The same goes for orElseGet, that you use as if you were a else (in which the if equivalent is the map), and returns null, only because something has to be returned (being that orElseGet serves to return a value default if the findAny found nothing).

A simpler way is to get the result of findAny, which is a Optional - that is, it represents a value that may or may not exist. Then just check whether the value exists or not, and take the respective actions in each case:

public void adicionarItem(Produto produto, BigDecimal valorUnitario, int quantidade) {
    Optional<Item> opt = itens.stream().filter(item -> item.getProduto().equals(produto)).findAny();
    if (opt.isPresent()) {
        opt.get().setQuantidade(opt.get().getQuantidade() + quantidade);
        opt.get().setValorUnitario(valorUnitario);
    } else {
        itens.add(new Item(produto, valorUnitario, quantidade));
    }
}

---

In the Java 9, you can use the method ifPresentOrElse, that receives the shares to be made if the value of the Optional exists or not. So it would look like this:

public void adicionarItem(Produto produto, BigDecimal valorUnitario, int quantidade) {
    itens.stream().filter(item -> item.getProduto().equals(produto)).findAny()
      .ifPresentOrElse( // este método é somente para Java >= 9
        item -> { // se já existe, atualiza os valores
            item.setQuantidade(item.getQuantidade() + quantidade);
            item.setValorUnitario(valorUnitario);
        },
        () -> { // se não existe, adiciona na lista
            itens.add(new Item(produto, valorUnitario, quantidade));
        }
      );
}

If you are using Java 8, you can implement something like this. But honestly, I think it’s an exaggeration to have to create an additional structure just to be able to do a single string. This is not always possible, or strictly necessary, and often not the best solution.

---

The algorithms above run through the list until you find a Item that has the Produto indicated. In the worst case, you will have to go through the entire list to see if the product is in it. If you want to improve this search, you can change the List for a java.util.Map.

How you used equals to compare products, I am assuming that methods equals and hashCode are properly and correctly implemented (see this question and this article for more details on these methods).

Basically, we can use a Map mapping products to their respective items:

Map<Produto, Item> map = new HashMap<>();

And the method for adding products would look like this:

public void adicionarItem(Produto produto, BigDecimal valorUnitario, int quantidade) {
    if (map.containsKey(produto)) {
        Item item = map.get(produto);
        item.setQuantidade(item.getQuantidade() + quantidade);
        item.setValorUnitario(valorUnitario);
    } else {
        map.put(produto, new Item(produto, valorUnitario, quantidade));
    }
}

// imprimir os itens
for (Item item : map.values()) {
    System.out.println(item);
}

I’m sorry to disappoint you, but for this case the best solution doesn’t use either streams. With the Map it is quite easy to check if there is already an item corresponding to the product being added. And I chose a HashMap as implementation, because according to documentation, makes the search in constant time (better than the linear search done in ArrayList).

Of course, if you want, you can also use stream:

Optional<Item> opt = map
    .entrySet()
    .stream()
    // encontrar chave igual ao produto
    .filter(entry -> entry.getKey().equals(produto))
    // obter o respectivo valor
    .map(Map.Entry::getValue)
    .findFirst();

But here we fall into the same problem as the previous solution. The result is a Optional, and to chain everything you would have to use ifPresentOrElse (for Java >= 9), or the solutions already proposed for Java 8 (the solution of this link or the code I suggested with isPresent()).

Honestly, I don’t think streams is the best solution for this case. And if it is to force everything into a single chain, worse. The ideal is to choose the most suitable solution for the problem, instead of trying to "fit" a solution that "I want/need to use", even if it is not the best option.