Reputation: 37594
Hello I probably oversaw something, but here it goes.
I have a TreeSet<CustomObject>
and I do not want to have duplicates in the Set. My CustomObject
class looks like this.
class CustomObject implements Comparable<CustomObject> {
String product;
String ean;
public CustomObject(String ean){
this.ean = ean;
// product is getting set via setter and can be null
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
CustomObject that = (CustomObject) o;
return ean.equals(that.ean);
}
@Override
public int hashCode() {
return ean.hashCode();
}
@Override
public int compareTo(CustomObject another) {
if(equals(another)) return 0;
if(product != null && another.product == null) return -1;
if(product == null) return 1;
return product.compareToIgnoreCase(another.product);
}
}
Now I have a add Function for new objects.
private final TreeSet<CustomObject> productCatalog;
public void addObject(SomeData tag) {
CustomObject p = new CustomObject(tag.getEan());
if (productCatalog.contains(p)) { // <-- This checks only one entry of the Set.
for (CustomObject temp : productCatalog) {
if (temp.equals(p)) {
p = temp; // I do stuff with that later which is irrelevent here
}
}
} else {
productCatalog.add(p);
}
}
The method productCatalog.contains(p)
calls the compareTo
method from the Comparable
Interface and does the comparing. The issue here is that it literally only checks I think the last object? in the set. So what happens is that only one unique CustomObject entry is present.
This is the scenario when I follow it with the debugger:
productCatalog.contains(p)
compareTo
equals
to check if ean.equals(that.ean)
How can I have it to check not only one object in step 4, but all the present objects in the Set. What am I missing?
Thx!
EDIT: These are some sample data. For the simplicity SomeData tag
is basically a String.
First run:
addObject("Ean1") // success added
addObject("Ean2") // success added
addObject("Ean3") // success added
addObject("Ean4") // success added
Everything gets added into the TreeSet.
Second run:
addObject("Ean1") // failed already in the map
addObject("Ean2") // failed already in the map
addObject("Ean3") // failed already in the map
addObject("Ean5") // success added
addObject("Ean4") // success added
addObject("Ean4") // success added
For testing purpose I manually set product names depending on the String ean
.
public CustomObject(String ean){
this.ean = ean;
switch(ean){
case "Ean1": product = "TestProduct"; break;
case "Ean2": product = "ProductTest";break;
case "Ean3": product = "Product";break;
}
The TreeSet
acts as a cache.
Edit2: This is how I solved it.
for (CustomObject temp : productCatalog) {
if (temp.equals(p)) {
p = temp; // I do stuff with that later which is irrelevent here
}
}
I removed the if statement with the contains
method since that would always return ´1or
-1in my special case. Now I simply iterate over the Set to correctly use the
equals` method since the TreeSet uses compareTo() for checking every element in the Set.
The Java Docs state the following
Note that the ordering maintained by a set (whether or not an explicit comparator is provided) must be consistent with equals if it is to correctly implement the Set interface. (See Comparable or Comparator for a precise definition of consistent with equals.) This is so because the Set interface is defined in terms of the equals operation, but a TreeSet instance performs all element comparisons using its compareTo (or compare) method, so two elements that are deemed equal by this method are, from the standpoint of the set, equal. The behavior of a set is well-defined even if its ordering is inconsistent with equals; it just fails to obey the general contract of the Set interface.
Upvotes: 0
Views: 446
Reputation: 35805
The main problem:
compareTo
does return 1 if both product and other.product are null. This is wrong because they are actually equal. You probably forgot to set product names for the higher ean values, like "Ean4" and "Ean5".
Old answer:
Your implementations of equals
and compareTo
do not fit together.
equals
works on the ean and compareTo
on the product. This only works if you implicitly assume that equal ean imply equal product. If this is not true in your test cases, the result will be wrong.
In either case, it is no good implementation because this can lead to a < b, b < c but a "equals" c.
Upvotes: 1