6

I have simple question to you, I have class Product that have fields like this:

private Integer id;
private String category;
private String symbol;
private String desc;
private Double price;
private Integer quantity;

I want to remove duplicates item from LinkedHasSet based on ID, e.g Products that have same ID but diffrent quantity will be add to set, I want to remove (update) products with same ID, and it will by my unique id of object, how to do that?

e.g Product: id=1, category=CCTV, symbol=TVC-DS, desc=Simple Camera, price=100.00, quantity=1 Product: id=1, category=CCTV, symbol=TVC-DS, desc=Simple Camera, price=100.00, quantity=3

won't be added to set

my code:

    public void setList(Set<Product> list) {
    if(list.isEmpty()) 
        this.list = list;
    else {
        this.list.addAll(list);
        Iterator<Product> it = this.list.iterator();
        for(Product p : list) {
            while(it.hasNext()) {
                if(it.next().getId() != p.getId())
                    it.remove();
                    this.list.add(p);   
            }
        }
    }
}
insict
  • 733
  • 2
  • 10
  • 19
  • 1
    I didn't really understand: the set can only contain products that have different IDs, whatever their other attributes (quantity, but also category, symbol, etc.), right? – sp00m Feb 07 '13 at 09:54
  • Yes, unique ID of object will be Product.id – insict Feb 07 '13 at 09:56
  • This all about your `equals()` and `hashCode()` implementation. try to override these methods in a proper way. – subodh Feb 07 '13 at 10:08

5 Answers5

17

All Set implementations remove duplicates, and the LinkedHashSet is no exception.

The definition of duplicate is two objects that are equal to each other, according to their equals() method. If you haven't overridden equals on your Product class, then only identical references will be considered equal - not different instances with the same values.

So you need to add a more specific implementation of equals (and hashcode) for your class. For some examples and guidance, see Overriding equals and hashcode in Java. (Note that you must override hashcode as well, otherwise your class will not behave correctly in hash sets.)

Community
  • 1
  • 1
Andrzej Doyle
  • 97,637
  • 30
  • 185
  • 225
  • I have equal and hashCode methods generated by eclipse, so I must use Obj.equal(Obj) style ? – insict Feb 07 '13 at 09:55
  • 1
    No, the LinkedHashSet will call `equals` itself internally, and will not insert an element that is equal to one already in the set. If you're seeing "duplicates" in your set, then the equals method must not be defined how you expect - you must have two duplicates `a` and `b` such that `a.equals(b)` returns false. (Or, since it's a hash-based set, `a.hashCode() != b.hashCode()`) – Andrzej Doyle Feb 07 '13 at 10:26
  • You can retain the default overridden hashcode implementation from eclipse, your Product class should have the logic of returning false when the productId is equal when compared with another object, and all other properties can be left unchecked for equality... Dont forget to handle transitive, reflexive and symmetric equlality for objects – Punith Raj Feb 07 '13 at 10:40
  • but how to do that when in equals method signature I must have Object type, and compiler don't know that it will be Product. I can not use generic in method signature because it won't be overridden – insict Feb 07 '13 at 10:45
  • @user1853125 That's how the method is defined, because you can compare equality against any object. Typically, implementations will check whether the other object is the same class - if not, they immediately return `false`, otherwise they cast to `Product` and go on to compare fields. If you have a look at start of the Eclipse-generated code, it's almost certainly doing something like this. You just need to change the logic that compares fields. – Andrzej Doyle Feb 07 '13 at 10:52
  • ok, I wrote something like this: `@Override public boolean equals(Object obj) { if(!(obj instanceof Product)) return false; Product p = (Product) obj; if(this.id.hashCode() == p.id.hashCode() ) { return true; } else { return false; } }` and its works properly – insict Feb 07 '13 at 10:55
  • strange, equals works, but LinkedHashSet still adds same-id objects ;/ – insict Feb 07 '13 at 11:14
  • 3
    @user1853125 Your `equals()` method is implemented wrong. You should compare object's fields, not the `hashCode()` result. There is a chance that completely different objects return the same hash code. You should first learn how hashtables work and why it is important to implement `equals()` and `hashCode()` properly. Irrespective of that, even if you implement `equals()` properly, you've got design issues in the code, which will cause other problems later – Adam Dyga Feb 07 '13 at 13:32
2

I won't give you straight answer, but a couple of advices.

  1. If you want to put Product in a Set you need to implement its equals() and hashCode() methods.
  2. When implementing equals() you will have to decide what "equality" for a Product means, (Set can contain only one instance in terms on "equality"). For instance, if two Product instances are "equal" is it enough if they have the same ID or should we also take quantity into account?. It's not easy to answer this question in your case, but please read on.
  3. Normally in situation like this only ID is taken into account. In that case you shouldn't have two Product instances in memory with different quantities, because one of them would represent a state that is incorrect (ie. particular product's quantity can be either 1 or 3, not both at a time).

  4. I think the design is not fully correct. Product class in your case represents a general product description (including price), so quantity doesn't really fit there. If someone can order a couple of copies of Product I think you should create another class such as Order or OrderLine which specifies what product is ordered and the corresponding quantity, eg:

    class OrderLine {
  private Product product;
  private Integer quantity; 
}

    With design like this it's easy to answer question from point 2. Product.equals() should compare the ID only, and OrderLine.equals() both the product (ID) and quantity.

Adam Dyga
  • 8,056
  • 3
  • 24
  • 34
  • so I must create `Order` class with simple setter and getter methods and adds overriden hashCode and equals methods? – insict Feb 07 '13 at 14:46
  • I use Product class with quantity because I use it in Datatable component with selector column, so it quite difficult to implement it by using two class. – insict Feb 07 '13 at 14:50
  • @user1853125 I don't fully understand what "Datatable component with selector column" is, but if you have just single table which contains both the product description, price and quantity, then the DB design is also wrong and it's probably the root of your problems. You should have one table for Product (and its descriptions) and another one for Orders (that has a foreign key to Product, and a quantity column). The Order usually has its own ID too. – Adam Dyga Feb 07 '13 at 15:09
  • but when I have OrderLine class i must to create new `equals` and `hashCode` methods to define unique ID, that's really complicated – insict Feb 08 '13 at 12:12
1

I would suggest you implement your own hash function in a way that it hashes elements with equal ID-s with the same code. This will solve your issue, without you having to code it explicitly.

Ivaylo Strandjev
  • 64,309
  • 15
  • 111
  • 164
0

The code seems to be adding to the list twice. Once during the addAll() call then once again during the iteration. In this case I believe the second iteration would suffice. The comparison should also be modified to use equals instead of ==

public void setList(Set<Product> list) {
    if(list.isEmpty()) 
        this.list = list;
    else {
        //this.list.addAll(list); Do not add all
        Iterator<Product> it = this.list.iterator();
        for(Product p : list) {
            while(it.hasNext()) {
                if(it.next().getId().equals(p.getId()))
                {
                    this.list.add(p);
                }   
            }
        }
    }
}
Kevin Bowersox
  • 88,138
  • 17
  • 142
  • 176
0

As others have already said, you will need to implement (override) equals(), hashCode() and (prefarably) compareTo() from Comparable interface. These methods, if not implemented correctly can lead to unexpected runtime behavior. Hard to debug issues. Hence, I suggest you use Apache Commons EqualsBuilder, HashcodeBuilder and ComparableBuilder to implement these methods. An example of how to use Apache Commons builder can be seen in this link http://www.javaworld.com/community/node/1859

Bimalesh Jha
  • 1,402
  • 9
  • 13