Java TreeSet: remove and contains() not working

Question

I have added some simple objects to a TreeSet, but when I call the TreeSet's remove() and contains() methods, they don't work. However, when I iterate over the set the object is printed. Employee objects shall be added to the set while the objects uniqueness is based on the objects name property. The Id property is the value that should be sorted, but which is not unique.

public class Employee {
    private String name;
    private int id;

    public int getId() {
        return id;
    }

    public void setId(int id) {
        this.id = id;
    }

    public String getName() {
    return name;
    }

    public void setName(String name) {
    this.name = name;
    }

 // Two objects are considered equal if their names are equal
    @Override
    public boolean equals(Object o) {
    if (o == null)
        return false;
    if (this == o)
        return true; 
    if (o.getClass() == this.getClass()) {
        Employee p = ( Employee) o;
        if (p.getName() != null && this.getName() != null)
        return this.getName().equals(p.getName());
        else
        return false;
    } else {
        return false;
    }
    }
} 

//*******************************************************

public class EmployeeComp implements Comparator<Employee> {

    // Sort Ids, but allow duplicates, hence this function is never returning 0
    @Override
    public int compare(Employee p1, Employee p2) {
    int re = 0;

    boolean scoreLt = (p1.getId() > p2.getId());
    boolean scoreGt = (p1.getId() < p2.getId());

    if(scoreLt)
        re = -1;
    if(scoreGt)
        re = 1;
    else 
        re = -1;                       
         return re;                 
    }    
}
//*******************************************************
// Collection shall store unique names with ordered values like:
// Alex, 923
// Toni, 728
// Eddi, 232
// Peter, 232
// Eddi, 156  *** not allowed
import java.util.TreeSet;


public class Main {
    private static EmployeeComp comp = new EmployeeComp(); 
    private static TreeSet<Employee> employees = new TreeSet<Employee>(comp); 

    public static void main(String[] args) {

    Employee p1 = new Employee();
    p1.setName("Eddi");
    p1.setId(232);

    Employee p2 = new Employee();
    p2.setName("Toni");
    p2.setId(728);

    Employee p3 = new Employee();
    p3.setName("Peter");
    p3.setId(232);

    Employee p4 = new Employee();
    p4.setName("Alex");
    p4.setId(923);

    employees.add(p1);
    employees.add(p2);
    employees.add(p3);
    employees.add(p4);

    // Here, contains() and remove() should check the object address
    // and not perform their actions based on compareTo

       } 
}

Answer 1

A TreeSet inserts/removes according to the results of Comparable, not .equals()/.hashCode()!

This means, BTW, that the objects of your Set do implement Comparable (if they didn't, each time you'd have tried and inserted a member, you'd have been greeted with a ClassCastException).

To be more accurate, TreeSet is an implementation of SortedSet.

If you want a .equals()/.hashCode()-compatible set, use, for instance, a HashSet.

For the illustration, here is what happens with BigDecimal (posted a few hours ago here):

final BigDecimal one = new BigDecimal("1");
final BigDecimal oneDotZero = new BigDecimal("1.0");

final Set<BigDecimal> hashSet = new HashSet<>();
// BigDecimal implements Comparable of itself, so we can use that
final Set<BigDecimal> treeSet = new TreeSet<>();

hashSet.add(one);
hashSet.add(oneDotZero);
// hashSet's size is 2: one.equals(oneDotZero) == false

treeSet.add(one);
treeSet.add(oneDotZero);
// treeSet's size is... 1! one.compareTo(oneDotZero) == 0

To quote the javadoc for Comparable, it means that BigDecimal's .compareTo() is "not consistent with .equals()".

** EDIT ** As to what the OP wants:

• a Collection which will not accept duplicate names;
• a sorted view of that Collection which will sort against the user's id.

As mentioned above, you cannot have one collection which does both. The solution:

• for the first, a HashSet;
• for the second, a copy of that set into an ArrayList, then using Collections.sort().

This means .equals() and .hashCode() must act only on the name, while a custom Comparator will act on the id. The Comparator has no other choice but to be custom since it is a comparator which is not consisten with .equals() in any event.

As to the proposed code, there are problems.

First: Employee overrides .equals() but not .hashCode(). As such, Employee breaks the .equals() contract (one part of which is that if two objects are equal, they must have the same hashcode). What is more, .hashCode() is critical for HashSet to work at all. Fix:

@Override
public int hashCode()
{
    return name == null ? 0 : name.hashCode();
}

@Override
public boolean equals(final Object obj)
{
    if (obj == null)
        return false;
    if (this == obj)
        return false;
    if (!(obj instanceof Employee))
        return false;
    final Employee other = (Employee) obj;
    return name == null ? other.name == null
        : name.equals(other.name);
}

Second: the comparator is equally as broken as Employee since it breaks the Comparator contract (for any o1 and o2, o1.compareTo(o2) == - o2.compareTo(o1)). Fix:

public final class EmployeeComp
    implements Comparator<Employee>
{
    @Override
    public int compare(final Employee o1, final Employee o2)
    {
        final int id1 = o1.getId(), id2 = o2.getId();
        if (id1 == id2)
            return 0;
        return id1 > id2 ? 1 : -1;
    }
}

Then, how to obtain a sorted copy of the set:

// "set" contains the unique employees
final List<Employee> sorted = new ArrayList<Employee>(set);
Collections.sort(list, new EmployeeComp());

DONE.

Answer 2

Your problem is conceptual.

If you want a sorted collection of unique objects: TreeSet
If you want a sorted collection were different objects can have the same comparison value for sorting purposes: PriorityQueue

Incidentally, the methods in a PriorityList are more suited to the usual needs of the second case than the ones on TreeSet. I used to think of it as TreeSet shortcomings. For example, to take the first item out of the collection.

Hope that helps :-)