3

I have a problem that seems to be trivial but I'm looking for the best way of resolving that issue.

Let's say I have a class:

Product.class

public class Product {
    private int id;
    private String code;
    private String price;
    private String quantity;

    public Product() {}

    //getters and setters

    @Override
    public boolean equals(Object obj) {
        boolean result = false;
        if (obj instanceof ProductOnStockDto) {
            ProductOnStockDto product = (ProductOnStockDto) obj;
            result = (this.code.equals(product.getCode()) && this.price.equals(product.getPrice()));
        }
        return result;
    }
}

And let's say I have a list of this objects:

List<Product> products;

Which is filled with data like this(1 row is one list element):

And what I need is to iterate over this list and squash elements that have the same code and the same price. For example:

In the result list I would like to have two products for code 3:

code 3 | 1.22 | 11
code 3 | 2.20 | 6

So I would do sth like this:

for(Product product : products) {
    for(Product productToSquash : products) {
        if(product.getId() != productToSquash.getId() && product.equals(productToSquash)) {
            //Here I would like to squash these two rows
            //Data conversion is ommited
            product.setQuantity(product.getQuantity() + productToSquash.getQuantity());
            //after that I would like to remove productToSquash object from the collection
        }
    }
}

But as I know it is not allowed to modify collection that I'm iterating on. So what would be the best way to squash all product list according to the example?

Tsung-Ting Kuo
  • 1,171
  • 6
  • 16
  • 21

2 Answers2

1

Load them to the HashMap to "squash" and then load from it to the new collection

    class Pair
    {
         String code;
         float price;
    }

    HashMap<Pair, Product> hashMap = new HashMap<Pair, Product>();
    List<Product> collection = new ArrayList<Product>(); //a new container for your values

    //for every Product in products
    //    if in hashMap there is no Product with code and price add it
    //    else resolve it (override? ignore? ...)

    for(Entry<Pair, Product> entry : hashMap.values()) {
        collection.add(entry.getValue());
    }

Notice that you have to decide how exactly will you be squashing elements

m.antkowicz
  • 13,268
  • 18
  • 37
  • 2
    Can't do `new List`. Did you mean `new ArrayList`? --- Can't iterate `hashMap`. Did you mean `hashMap.entrySet()`? --- If you did, why not use `hashMap.values()`? --- You need to `clear()` the collection before adding the values back. --- Original order of values are lost. – Andreas Dec 19 '15 at 00:32
  • Hi Andreas, thank you for your advices, this code is kind of pseudo but of course I should care about proper syntax - I'm not sure if I get *clear()* thing - can you please explain? – m.antkowicz Dec 19 '15 at 00:39
  • You said "for every Product in `collection`", which presumes some code initially filling the `collection` list, then iterating that list to build `hashMap`, and finally adding values back to `collection`. If you don't clear the list before adding "merged" products back, you'd end up with lots of duplicates, except the ones you "squashed". – Andreas Dec 19 '15 at 00:42
  • Oh now I get it - no I mean **collection** object to be new collection for items from origin collection – m.antkowicz Dec 19 '15 at 00:44
  • Thank you. I did exactly according to the pseudocode – Daniel Urbaniak Dec 23 '15 at 21:08
  • Glad to help you - if answer is appropriate for you request you can accept it - [read more here](http://stackoverflow.com/help/accepted-answer) – m.antkowicz Dec 24 '15 at 00:03
1

First of all, your equals() method refers to ProductOnStockDto. That should be Product.

To remove elements during iteration, use the Iterator directly, i.e. use an "old style" for-loop, and use a Map<Product, Product> to keep track of products previously seen. This requires that you also implement hashCode():

@Override
public int hashCode()
{
    return this.code.hashCode() * 37 + this.price.hashCode();
}
Map<Product, Product> map = new HashMap<Product, Product>();
for (Iterator<Product> productIter = products.iterator(); productIter.hasNext(); ) {
    Product product = productIter.next();
    Product productToKeep = map.get(product);
    if (productToKeep == null)
        map.put(product, product);
    else {
        productToKeep.setQuantity(productToKeep.getQuantity() + product.getQuantity());
        productIter.remove();
    }
}

You shouldn't really be doing it this way, since the equals() method is returning true for objects that are not truly equal.

You should instead have a key class with just code and price, and the key class is one that needs to implement equals() and hashCode(), not Product.

Andreas
  • 154,647
  • 11
  • 152
  • 247