The TreeSet relies on your Patient/PatientImpl equals/hashCode/compareTo methods. They should be correctly implemented according to Object/Comparable contracts.
– Ale ZalazarMar 2 '14 at 3:13

1

You have omitted the most important part, which is the code for the Patient class. If Patient does not contain correct implementations of equals() and hashCode() it will not work.
– Jim GarrisonMar 2 '14 at 3:13

1 Answer
1

If you put your objects in a TreeSet, you need to either provide an implementation of the Comparator interface in the constructor, or you need your objects to be of a class that implements Comparable.

You said you implement compareTo from the Comparable interface, but in your comment you say that you didn't, so am I correct in assuming that you just return 0; in the compareTo method? That would explain your problem, because TreeSet would then think that all your objects are 'the same' based on the compareTo method result.

Basically, in a TreeSet, your objects are maintained in a sorted order, and the sorting is determined by the outcome of the Comparable/Comparator method. This is used to quickly find duplicates in a TreeSet and has the added benefit that when you iterate over the TreeSet, you get the results in sorted order.

The Javadoc of TreeSet says:

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.

The easiest way to achieve that is to let your equals method call the compareTo method and check if the result is 0.

Given your PatientImpl class, I assume that you would want to sort patients first by their last name, then by their first name, and then by the rest of the fields in the class.

I believe that would solve the problem you described.
I would advise you to read up (Javadoc or books) on TreeSet and HashSet and the importance of the equals, compareTo and hashCode methods.
If you want to put your objects in a Set or a Map, you need to know about these to implement that correctly.

Note
I based this compareTo method on your equals method.
You were comparing the date-or-birth by first calling toString. That's not a very good way of doing that - you can use the equals method in java.util.Date directly. In a compareTo method the problem gets worse because dates do not sort correctly when you sort them alphabetically.
java.util.Date also implements Comparable so you can replace that comparison in the method with:

if (r == 0)
r = this.dob.compareTo(temp.getDOB());

In addition, if any of the fields could be null, you need to check for that as well.

Erwin, I appreciate you taking your time for that thorough answer. It really helped. And you were right on several notes, specifically - when I implemented the Comparable interface in my PatientImpl class, I let it add the compareTo method automatically (with return 0), which made it seem like everything was equal. I changed that, and now it works! Thanks for your points about the Date, and everything else! (I would plus 1 your answer if I had enough reputation points!) Cheers
– JonaMar 2 '14 at 4:32

The compareTo implementation returns 0 in the end. It should be r, right?
– jayeffkayMar 21 '17 at 9:36

1

@jayeffkay You're right, funny that this wasn't caught for 3 years. I've also updated the comparison of the id's - using Integer.compare is better than subtraction because it avoids possible integer overflow conditions.
– Erwin BolwidtMar 21 '17 at 9:58