Java Collections.sort() not sorting as expected

Question

I am trying to sort two different ArrayLists of objects by a specific atribute ('Student' objects by "program" and 'Professor' objects by "faculty"). Both classes extend my abstract 'Person' class.

public abstract class Person implements Comparable<Person>{
    private String name;
    private String adress;

    //getters, setters, etc., all works properly

    @Override
    protected Object clone() throws CloneNotSupportedException {
        return super.clone(); 
    }

    public int compareTo(String string) {
        return name.compareTo(string);
    }
}

Then, when I create an array of 1000000 random 'Person' objects than can be Students or Professors, I decide to sort it alphabetically by their name like this (which works properly).

Person personByName[] = arrayPersonas.clone();
Arrays.sort(personByName);

Then, I divide the original Person array into two ArrayLists, one for Student objects and another for Professor objects:

    ArrayList<Student> studentsByProgram = new ArrayList();
    ArrayList<Professor> professorsByFaculty = new ArrayList();
    for (int i = 0; i < 1000000; i++) { 
        if (arrayPersonas[i] instanceof Student) {
            studentsByProgram.add((Student)arrayPersonas[i]);
        } else {
            professorsByFaculty.add((Professor)arrayPersonas[i]);
        }
    }

The problem comes when i try to sort each ArrayList alphabetically by the atribute I want, since it keeps sorting them by the name of the Person:

Collections.sort(studentsByProgram);
Collections.sort(professorsByFaculty);

Here I leave my Student and Professor classes:

public class Student extends Person {
    private String program;
    private int year;
    private double fee;

    //constructor, setters, getters, toString, equals

    @Override
    protected Object clone() throws CloneNotSupportedException {
        return super.clone(); 
    }



    public int compareTo(String string) {
        return program.compareTo(string); 
    }

    @Override
    public int compareTo(Person t) {
        return super.compareTo(t.getName());
    }
}

Professor class:

public class Professor extends Person {
    private String faculty;
    private double salary;

    //constructor, setters, getters, toString, equals

    @Override
    protected Object clone() throws CloneNotSupportedException {
        return super.clone(); 
    }


    public int compareTo(String string) {
        return faculty.compareTo(string); 
    }

    @Override
    public int compareTo(Person t) {
        return super.compareTo(t.getName());
    }
}

What am I doing wrong? I thought if I call "Collections.sort()" on an ArrayList of Student objects it would use the "compareTo()" method from my Student class, which uses the "program" atribute. I'm still learning to work with these methods so there is something I'm not getting.

Answer 1

You have two distinct compareTo() methods. The one you're expecting to be used does not get invoked by Collections.sort().

If you want orderings on Students using Collections.sort() then you need a method with signature compareTo(Student student);

This method "overlaps" with compareTo(Person person) and that's a problem on two counts :

• semantically, the compareTo() method at Person level establishes semantics and your compareTo() method at the Student level deviates from those semantics and that's never a good idea.

• technically, you are relying on implementation details related to the method binding to make your system behave as desired. That's dodgy at best.

I'd look out for a sorting method that uses an explicit user-provided comparator instead of a sorting method that relies on internal compareTo().

Answer 2

The problems

1. You didn't define how Person objects should be compared.
2. You incorrectly defined how Student and Professor instances should be compared.
3. You wrote overloaded methods compareTo(String) that are misleading.

The solutions

1. Define Person#compareTo properly, remove its compareTo(String):

   public int compareTo(Person p) {
       return getName().compareTo(p.getName());
   }
   

2. Define Student#compareTo and Professor#compareTo correctly, remove their compareTo(String). Here's an example of how Student#compareTo could be written:

   @Override
   public int compareTo(Person t) {
       final int personComparisonResult = super.compareTo(t);
   
       if (personComparisonResult == 0) {
           return program.compareTo(((Student) t).program);
       }
   
       return personComparisonResult;
   }
   

   It says "compare them as Persons first; if they are equal (here, have the same name), compare them as Students (here, by student's program)".

3. I would remove these methods. It isn't worth having a separate method for a simple line of code which doesn't fit the class domain.