Java Priority Queue with 2 Priority parameters

Question

I have a class Person which implements Comparable as shown below because I want to put objects of Person in a priority queue.

public class Student implements Comparable{
    private String fullName;
    private Date registrationDate;

public Person(String fullName){
    this.fullName = fullName;
}

public String getFullName() {
    return fullName;
}

public void setFullName(String fullName) {
    this.fullName = fullName;
}

public Date getRegistrationDate() {
        return registrationDate;
    }
    public void setRegistrationDate(Date registrationDate) {
        this.registrationDate = registrationDate;
    }

@Override
    public int compareTo(Object obj) {
        Person person = (Person) obj;

        if(person instanceof Staff){
            return 1; 
        }else if(person instanceof Student){
            return -1;
        }
        else if(getRegistrationDate().before(person.getRegistrationDate())){
            return 1;
        }else if(getRegistrationDate().after(person.getRegistrationDate())){
            return -1;
        }
        return 0;
    }
}

I have two classes that extend Person as follows

public class Staff extends Person{
    public Staff(String fullName){
        this.fullName = fullName;
     }
}

public class Student extends Member{
    public Student(String fullName){
        this.fullName = fullName;
     }
}

In the main method, I am creating objects of Staff and objects of Students, setting a registration date of the objects and putting them in a priority queue

public class School {

    public static void main(String args[]) {

        //list of students
        Student student1 = new Student("John Kent");
        Date dateStudent1Joined = new GregorianCalendar(2014, Calendar.JULY, 1).getTime();
        student1.setRegistrationDate(dateStudent1Joined);

        Student student2 = new Student("Peter Tush");
        Date dateStudent2Joined = new GregorianCalendar(2014, Calendar.JULY, 2).getTime();
        student2.setRegistrationDate(dateStudent2Joined);

        Student student3 = new Student("Mike Monroe");
        Date dateStudent3Joined = new GregorianCalendar(2014, Calendar.JULY, 3).getTime();
        student3.setRegistrationDate(dateStudent3Joined);

        Student student4 = new Student("Tom Johnson");
        Date dateStudent4Joined = new GregorianCalendar(2014, Calendar.JULY, 4).getTime();
        student4.setRegistrationDate(dateStudent4Joined);

        Student student5 = new Student("Tony Spencer");
        Date dateStudent5Joined = new GregorianCalendar(2014, Calendar.JULY, 5).getTime();
        student5.setRegistrationDate(dateStudent5Joined);

        //list of staff
        Staff staff1 = new Staff("Luke Clint");
        Date dateStaff1Joined = new GregorianCalendar(2014, Calendar.JULY, 6).getTime();
        staff1.setRegistrationDate(dateStaff1Joined);

        Staff staff2 = new Staff("Ron West");
        Date dateStaff2Joined = new GregorianCalendar(2014, Calendar.JULY, 7).getTime();
        staff2.setRegistrationDate(dateStaff2Joined);

        Staff staff3 = new Staff("Jim Gary");
        Date dateStaff3Joined = new GregorianCalendar(2014, Calendar.JULY, 8).getTime();
        staff3.setRegistrationDate(dateStaff3Joined);


        //create a queue data structure to hold Persons in school
        PriorityQueue<Person> schoolQueue = new PriorityQueue<Person>();
        //add students to queue
        schoolQueue.offer(student1);
        schoolQueue.offer(student2);
        schoolQueue.offer(student3);
        schoolQueue.offer(student4);
        schoolQueue.offer(student5);
        //add staff to queue
        schoolQueue.offer(staff1);
        schoolQueue.offer(staff2);
        schoolQueue.offer(staff3);

        //print names of people in queue
        for(Member member : clubQueue){
            String memberName = member.getFullName();
            System.out.println(memberName);
        }

    }
}

My priority queue should follow 3 rules 1.Staff objects should have a higher priority than student objects 2.Staff with an earlier registration date should have a higher priority than staff with a later registration date 3.Students with an earlier registration date should have a higher priority than students with a later registration date.

The output I am getting does not yield the desired results at the moment. The staff objects have higher priority than the student objects but the priorities according to date is not working. I understand that the rules in my compareTo method is where the issue lies, how might I improve it? This is it again for convenience

@Override
        public int compareTo(Object obj) {
            Person person = (Person) obj;

            if(person instanceof Staff){
                return 1; 
            }else if(person instanceof Student){
                return -1;
            }
            else if(getRegistrationDate().before(person.getRegistrationDate())){
                return 1;
            }else if(getRegistrationDate().after(person.getRegistrationDate())){
                return -1;
            }
            return 0;
        }

And you're still not using generics. Sigh... Ask yourself: what is the result of your compare() method when comparing staff1 with staff2 with staff1.compareTo(staff2)? And when comparing staff2 with staff1? Does it seem correct to you? Hint: add a int getPriority() method to your Person class, and override it in the two classes. Then compare by priority, then registration date. Without any instanceof. — JB Nizet
– JB Nizet, Commented Aug 29, 2015 at 9:17

JB Nizet · Accepted Answer · 2015-08-29 09:34:21Z

Let's start by making your Person class use generic types instead of raw types:

public class Person extends Comparable<Person>

Then let's assign a priority to classes. You didn't tell how "bare" persons, which are neither staff nor student, should compare with other ones. I'll thus assume that "bare" persons shouldn't exist, and that your Person class should thus be abstract:

public abstract class Person extends Comparable<Person>

Then you want each class to have a priority. So let's implement that, instead of relying on ugly instanceof:

protected abstract int getPriority();

A Staff should come before a Student, so in Staff:

@Override
protected int getPriority() {
    return 0;
}

and in Student:

@Override
protected int getPriority() {
    return 1000;
}

Now let's implement the compareTo method:

public int compareTo(Person other) {
    int result = Integer.compare(this.getPriority(), other.getPriority());
    if (result == 0) {
        result = this.getRegistrationDate().compareTo(other.getRegistrationDate())
    }
    return result;
}

Note that adding another kind of Person is trivial. You just need to return the appropriate value in getPriority, and the comparison code won't have to be changed.

Also notice that compareTo now takes a Person as argument, and that the compiler will now prevent you from doing silly things like person.compareTo("foo"), because of the correct generic type.

If using Java 8, your compareTo method could be even simpler:

private static final Comparator<Person> COMPARATOR = 
    Comparator.comparingInt(Person::getPriority) 
              .thenComparing(Person::getRegistrationDate);

@Override
public int compareTo(Person other) {
    return COMPARATOR.compare(this, other);
}

Wololo · Accepted Answer · 2015-08-29 09:52:56Z

NOTE: Comparable is a generic Interface. Put generics this to your use.

Lets take a look at your compareTo method. The compareTo method first checks weather obj is instanceof Staff or Student.

schoolQueue contains Staff and Student objects. Either:

if(person instanceof Staff){
  return 1;

or:

}else if(person instanceof Student){
  return -1;
}

is executed. So the last two else if blocks are never executed.

Solution:

// If the object on which compareTo() is applied is of type Student and person
// is of type Staff, return -2
// -2 indicates that Staff has priority over student
if(this instanceof Student && person instanceof Staff){
  return -2; 
}
// If the object on which compareTo() is applied is of type Staff and person
// is of type Student, return 2
else if(this instanceof Staff && person instanceof Student){
  return 2;
}
// If both are of same type, prioritize the one who registered early
else {
  return (this.getRegistrationDate()).compareTo(person.getRegistrationDate());
}

Peter Lawrey · Accepted Answer · 2015-08-29 09:17:41Z

0

One issue is that you always assume one Person is a Staff member and one is a Student. It might be that both are the same class.

 if(this instance Student && person instanceof Staff) {
        return 1; // Student has priority
 } else if (this instance Staff && person instanceof Student){
        return -1; // Student has priority
 }
 return getRegistrationDate().compareTo(person.getRegistrationDate());

answered Aug 29, 2015 at 9:17

Peter Lawrey

535k83 gold badges770 silver badges1.2k bronze badges

Collectives™ on Stack Overflow

Java Priority Queue with 2 Priority parameters

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related