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.

so, what is the functionality as it occurs? are you sorting in a wrong order? — Stultuske, Oct 01 '18 at 09:12
"What am I doing wrong?" Don't know - what happens that you don't expect? — Andy Turner, Oct 01 '18 at 09:12
"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:" — Marc Brauer, Oct 01 '18 at 09:13
In `Student` and `Professor` you have `compareTo()` with String as argument. You need to change that to `Student` and `Professor` respectively. — Turamarth, Oct 01 '18 at 09:14
@Turamarth did that, it still sorts the ArrayList by the 'name' atribute — Marc Brauer, Oct 01 '18 at 09:19
"it still sorts the ArrayList by the 'name' atribute" What are you expecting it to do? All of your comparison methods use the name. — Michael, Oct 01 '18 at 09:26
btw: You've override `clone()` in all three classes shown but no class implements `Cloneable`. As long as this interface isn't impemented cloning a person will throw `CloneNotSupportedException`. — LuCio, Oct 01 '18 at 09:41
funny that you have `@Override` for `clone`, but not for `compareTo` - that should have ringed a bell... — Eugene, Oct 01 '18 at 20:58

score 3 · Answer 1 · answered Oct 01 '18 at 09:25

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().

Andrew Tobilko · Accepted Answer · 2018-10-01T12:32:02.447

The problems

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

The solutions

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

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

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)".
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.

this worked perfectly with what I was trying to do, thank you Andrew — Marc Brauer, Oct 02 '18 at 09:26

Stephen C · Answer 3 · 2018-10-01T09:26:01.080

If you want to sort objects using different orderings to the classes "natural" ordering, you should be using Arrays.sort(T[], Comparator<T>), with a Comparator object that implements the specific sort ordering or orderings.

The javadoc for Comparable explains the semantics that it should implement. (Read them carefully!)

About natural ordering:

The "natural" ordering for a Person[] will given by the compareTo(Person) method.
The "natural" ordering for a Student[] (or ArrayList<Student>) will given by the compareTo(Student) method.
And so on.
In none of these case will your compareTo(String) methods be used!

score 0 · Answer 4 · answered Oct 01 '18 at 09:32

Your compareTo(String)method in class Person doesn't make much sense, because it compares this (a Person) with a String. Especially it does not contribute to the interface Comparable<Person> implemented by class Person.

You should rather compare this (a Person) with another Person:

@Override
public int compareTo(Person otherPerson) {
    return name.compareTo(otherPerson.name);
}

Then, in your classes Professor and Student you can use the above method like this:

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

Actually, this method is not needed anymore, because its behavior is the same as the compareTo(Person) of Person. You can omit this method, and still have the same effect.

score 0 · Answer 5 · answered Oct 01 '18 at 09:38

This is a good time to remind ourselves with Effective Java book Item 40: Consistently use the Override.

Your base class Person does not use the @Override notation on the compareTo method so you'll get no error if you're not actually overriding the compareTo method you think you are. In this case, the parameter type is wrong. It should be Person, not String. The method is not invoked and the default on is used instead.

-me

score 0 · Answer 6 · answered Oct 01 '18 at 09:59

I believe when you want to sort a Class over a specific attribute you need to use a Comparator.

Try something like:

static final Comparator<Student> compareProgram = new Comparator<Student>() {
        public int compare(Student e1, Student e2) {
            //condition ( you need to return the condition)
            return e2.program().compareTo(e1.program());

        }
};

// Employee database
static final Collection<Student> students = ... ;

public static void main(String[] args) {
    List<Student> e = new ArrayList<Student>(students);
    Collections.sort(e, compareProgram);
    System.out.println(e);
}

The comparator is a function that exists under the Collections so you just have to insert the condition that you are looking for.

Let me know if you are not able to implement it.

Java Collections.sort() not sorting as expected

6 Answers6

The problems

The solutions