How to remove oldest Student object from TreeSet?

Question

I'm trying to remove oldest Student from my TreeSet but the object is not getting deleted. Can someone please explain how to remove the oldest Student object from my TreeSet ?

public class Student implements Comparable<Student> {

    private String studentId;
    private String studentAge;

    public Student(String studentId, String studentAge) {
        super();
        this.studentId = studentId;
        this.studentAge = studentAge;
    }

    public String getStudentId() {
        return studentId;
    }

    public void setStudentId(String studentId) {
        this.studentId = studentId;
    }

    public String getStudentAge() {
        return studentAge;
    }

    public void setStudentAge(String studentAge) {
        this.studentAge = studentAge;
    }

    @Override
    public int compareTo(Student o) {
        if (Double.valueOf(studentAge).compareTo(Double.valueOf(o.getStudentAge())) == 0) {
            return 1;
        } else
            return Double.valueOf(studentAge).compareTo(Double.valueOf(o.getStudentAge()));
    }

    @Override
    public boolean equals(Object obj) {
        Student student = (Student) obj;
        return (student.getStudentId().equals(studentId) && student.getStudentAge().equals(studentAge));
    }

    @Override
    public String toString() {
        return "StudentId " + studentId + " Student Age " + studentAge;
    }

}


import java.util.TreeSet;

public class MainApp {

    public static void main(String[] args) {
        TreeSet<Student> studentList = new TreeSet<Student>();
        studentList.add(new Student("10001", "5"));
        studentList.add(new Student("10001", "15"));
        studentList.add(new Student("10001", "25"));
        studentList.add(new Student("10001", "3"));
        studentList.add(new Student("10001", "2"));
        studentList.add(new Student("10001", "1"));
        studentList.add(new Student("10001", "10"));
        studentList.add(new Student("10001", "6"));
        studentList.add(new Student("10001", "7"));
        studentList.add(new Student("10001", "2"));

        System.out.println(studentList.last().getStudentAge());
        System.out.println(studentList);
        Student maxAge = studentList.last();
        System.out.println(studentList.remove(new Student(maxAge.getStudentId(), maxAge.getStudentAge())));
        System.out.println(studentList);

    }

}

Answer 1

Your compare method is simply spoken plain wrong.

A first naive fix: extract the two ages values as number (and use integer, as age seems to always come as whole number). And then call

return Integer.compare(first, second);

only. ( maybe you will have to turn around the two values, depending how you want your set to be sorted )

Your current approach of returning 1 when the values are equal is just bogus.

The point is: the TreeSet can't properly sort the elements because your compareTo() method is logically broken. As soon as sort is working, you can easily identify the first or last element in the set and remove that one.

But the real issue here is: your underlying design is wrong. You see, when you have equals() and compareTo() people quickly talk about: should all equal efforts give 0 for compareTo()? This boils down to: what data goes into your equals() method. Think about it: does age the matter for the identity of two persons. No. You have a student ID. The nature of IDs is to uniquely identify things. So: you should really first clarify what makes up "unique" Students.

My suggestions:

• only the ID should be used, to uniquely identify Students. You can not have two students with the same student. You can not have two student objects with the same ID but different age. Because the ID is the student.
• from there, I would drop the Comparable interface from Student, and instead implement standalone Comparator<Student> instances. You could have one comparator that compares two Students to ascending age. And another one with descending.

That is the key point: you only use Comparable when you have a true "natural" sorting order, like for Integer. But persons, students: it depends on context how to sort them. Maybe ascending on age. But maybe you add a name later, and then you want to sort these name strings.

Answer 2

As others have said, your compareTo method is wrong. You should never return 1 when two instances are equal. This is a trivial fix. Having said that, your compareTo method is wrong not only because of this, but also because you are comparing students based only on their age. TreeSet is a Set, and sets can't have duplicate elements, so this means that your set won't be able to hold students that have the same age.

Luckily, each student has a unique ID, so you could use this id to break ties (i.e. when two students have the same age), thus making two instances different as per the compareTo method, even if they have the same age:

@Override
public int compareTo(Student o) {
    int result = Integer.compare(studentAge, o.getStudentAge());
    return result == 0 ? studentId.compareTo(o.getStudentId()) : result;
}

Note: this assumes the age is of type int.

Now, once you've fixed the compareTo method, you can use TreeSet's lookup methods safely. To remove the oldest student from your TreeSet, you can use the pollLast method, which does exactly what you want:

Student oldest = studentList.pollLast();

Answer 3

Few observations, if corrected will solve the issue.

-- Why you have age as String ? Please make it an Integer or Double. In that case your compareTo method will be

@Override
public int compareTo(Student o) {
        return this.studentAge.compareTo(o.getStudentAge());
}

-- If you still want to use the age as String use the below compareTo method.

@Override
public int compareTo(Student o) {
        return Double.valueOf(studentAge).compareTo(Double.valueOf(o.getStudentAge()));
}

Either of this will solve your problem.

---

Now the explanation: Unlike other maps(since sets are internally maps only, hope you know that), the keys are not compared based on equals method. The keys are compared based on compareTo if Comparable is implemented, or Compartor implementation. Since you compareTo method is buggy, the Student with age 25 is never found when the remove method was called.

When the remove method is called, it tries to search the key using the age, but since your compare technically never returns 0 (which means equal), so the object is never found hence never deleted.

But the last method gives the correct student value, because, the compareTo method in case of populating the values will work correctly..

Answer 4

As a suggestion,the equals method should consider the following conditions while overriding for .

1. If both the objects are pointing to same reference then they are equal by default, there is no need for content comparison
2. It should handle the ClassCastException and NullPointerException at run time, you can achieve this with instanceof operator.

added String class equals method for reference

  public boolean equals(Object anObject) {
    if (this == anObject) {
        return true;
    }
    if (anObject instanceof String) {
        String anotherString = (String)anObject;
        int n = value.length;
        if (n == anotherString.value.length) {
            char v1[] = value;
            char v2[] = anotherString.value;
            int i = 0;
            while (n-- != 0) {
                if (v1[i] != v2[i])
                    return false;
                i++;
            }
            return true;
        }
    }
    return false;
}