I have several arrays in the form:
private static String[] patientNames = { "John Lennon", "Paul McCartney", "George Harrison", "Ringo Starr" };
Then I make a TreeSet like this:
TreeSet<Patient> patTreeSet = new TreeSet<Patient>();
Where Patient is a different class that makes "Patient" objects.
Then I loop through each element in my arrays to create several patients and add them to my patTreeSet
like this:
for(int i = 0; i< patientNames.length; i++){
Date dob = date.getDate("MM/dd/yyyy", patientBirthDates[i]);
Patient p = new PatientImpl(patientNames[i], patientSSN[i], dob);
patTreeSet.add(p);
}
But when I go to check my patTreeSet.size()
it only returns "1" - why is this?
I know my objects are working well because when I try to do the same thing but with ArrayList
instead, everything works fine. So I'm guessing I'm using the TreeSet wrong.
If it helps, Patient has a method called getFirstName(), and when I try to do the following:
Iterator<Patient> patItr = patTreeSet.iterator();
while(patItr.hasNext()){
System.out.println(patItr.next().getFirstName());
}
Then only "John" prints, which obviously shouldn't be the case... So, am I totally misusing the TreeSet?
Thanks in advance for any help!
EDIT below
================PatientImpl Class====================
public class PatientImpl implements Patient, Comparable{
Calendar cal = new GregorianCalendar();
private String firstName;
private String lastName;
private String SSN;
private Date dob;
private int age;
private int thisID;
public static int ID = 0;
public PatientImpl(String fullName, String SSN, Date dob){
String[] name = fullName.split(" ");
firstName = name[0];
lastName = name[1];
this.SSN = SSN;
this.dob = dob;
thisID = ID += 1;
}
@Override
public boolean equals(Object p) {
//for some reason casting here and reassigning the value of p doesn't take care of the need to cast in the if statement...
p = (PatientImpl) p;
Boolean equal = false;
//make sure p is a patient before we even compare anything
if (p instanceof Patient) {
Patient temp = (Patient) p;
if (this.firstName.equalsIgnoreCase(temp.getFirstName())) {
if (this.lastName.equalsIgnoreCase(temp.getLastName())) {
if (this.SSN.equalsIgnoreCase(temp.getSSN())) {
if(this.dob.toString().equalsIgnoreCase(((PatientImpl) p).getDOB().toString())){
if(this.getID() == temp.getID()){
equal = true;
}
}
}
}
}
}
return equal;
}
and then all the getters are below, as well as the compareTo() method from the Comparable interface
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.
You could implement a compareTo
method like this:
@Override
public int compareTo(Object o) {
if (!(o instanceof Patient))
return -1;
Patient temp = (Patient) o;
int r = this.lastName.compareToIgnoreCase(temp.getLastName());
if (r == 0)
r = this.firstName.compareToIgnoreCase(temp.getFirstName());
if (r == 0)
r = this.SSN.compareToIgnoreCase(temp.getSSN());
if (r == 0)
r = this.dob.toString().compareToIgnoreCase(temp.getDOB().toString());
if (r == 0)
r = Integer.compare(this.getID(), temp.getID());
return r;
}
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-of-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.