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
– Stultuske, Commented Oct 1, 2018 at 9:12
"What am I doing wrong?" Don't know - what happens that you don't expect? — Andy Turner
– Andy Turner, Commented Oct 1, 2018 at 9: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
– Marc Brauer, Commented Oct 1, 2018 at 9:13
In Student and Professor you have compareTo() with String as argument. You need to change that to Student and Professor respectively. — Turamarth
– Turamarth, Commented Oct 1, 2018 at 9:14
@Turamarth did that, it still sorts the ArrayList by the 'name' atribute — Marc Brauer
– Marc Brauer, Commented Oct 1, 2018 at 9:19

Erwin Smout · Accepted Answer · 2018-10-01 09:25:42Z

3

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

answered Oct 1, 2018 at 9:25

Erwin Smout

18.5k4 gold badges36 silver badges54 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Andrew · Accepted Answer · 2018-10-01 12:32:02Z

1

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.

edited Oct 1, 2018 at 12:32

answered Oct 1, 2018 at 9:19

Andrew

49.8k15 gold badges99 silver badges145 bronze badges

1 Comment

Marc Brauer Over a year ago

this worked perfectly with what I was trying to do, thank you Andrew

Stephen C · Accepted Answer · 2018-10-01 09:26:01Z

1

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!

edited Oct 1, 2018 at 9:26

answered Oct 1, 2018 at 9:19

Stephen C

723k95 gold badges849 silver badges1.3k bronze badges

Comments

Thomas Fritsch · Accepted Answer · 2018-10-01 09:32:56Z

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.

Tatu Lahtela · Accepted Answer · 2018-10-01 09:38:01Z

0

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

answered Oct 1, 2018 at 9:38

Tatu Lahtela

4,57433 silver badges31 bronze badges

Comments

Nino Matos · Accepted Answer · 2018-10-01 09:59:35Z

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.

Collectives™ on Stack Overflow

Java Collections.sort() not sorting as expected

6 Answers 6

Comments

The problems

The solutions

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

The problems

The solutions

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related