Comparator Interface implemented in a nested class

Question

I'm new on stackoverflow.com but I often used it to search for answers whenever I had a problem, but now I can't find any result searching for my problem so I'm asking here :) I'm studying for the OCPJP SE 7 certification, exam 1Z0-804, and I'm using a book (there is only one available afaik, Ganesh\Sharma's one) In the collections chapter, about the Comparator interface, the book provide this example of using both the Comparator and the Comparable interface to sort an array of Student elements, but the question is about the Comparator:

import java.util.*;

class Student implements Comparable<Student> {
    private String id, name;
    private Double cgpa;
    public String getName() {
        return name;
    }
    public String getId() {
        return id;
    }
    public Double getCgpa() {
        return cgpa;
    }
    public Student(String studentId, String studentName, double studentCGPA) {
        id=studentId;
        name=studentName;
        cgpa=studentCGPA;
    }
    public String toString() {
        return id+" "+name+" "+cgpa;
    }
    public int compareTo(Student that) {
        return this.id.compareTo(that.id);
    }
}

class StudentCGPA implements Comparator<Student> {
    public int compare(Student s1, Student s2) {
        return s1.getCgpa().compareTo(s2.getCgpa());
    }
}

class MyMainClass {
    public static void main(String[] args) {
        Student[] students =    {   new Student("cs011", "Lennon", 3.1),
                                    new Student("cs021", "McCartney", 3.4),
                                    new Student("cs012", "Harrison", 2.7),
                                    new Student("cs022", "Starr", 3.7),
                                };
        Arrays.sort(students, new StudentCGPA());
        System.out.println(Arrays.toString(students));
    }
}

So it creates a new class only for using the Comparator interface with two Student objects, but I think this is very uncomfortable so I wonder: why can't I use a nested class (within Student)? Like this:

import java.util.*;

class Student implements Comparable<Student> {
    private String id, name;
    private Double cgpa;
    public String getName() {
        return name;
    }
    public String getId() {
        return id;
    }
    public Double getCgpa() {
        return cgpa;
    }
    public Student(String studentId, String studentName, double studentCGPA) {
        id=studentId;
        name=studentName;
        cgpa=studentCGPA;
    }
    public String toString() {
        return id+" "+name+" "+cgpa;
    }
    public int compareTo(Student that) {
        return this.id.compareTo(that.id);
    }
    static class StudentCGPA implements Comparator<Student> {
        public int compare(Student s1, Student s2) {
            return s1.getCgpa().compareTo(s2.getCgpa());
        }
    }
}

class MyMainClass {
    public static void main(String[] args) {
        Student[] students =    {   new Student("cs011", "Lennon", 3.1),
                                    new Student("cs021", "McCartney", 3.4),
                                    new Student("cs012", "Harrison", 2.7),
                                    new Student("cs022", "Starr", 3.7),
                                };
        Arrays.sort(students, new Student.StudentCGPA());
        System.out.println(Arrays.toString(students));
    }
}

The book says nothing about using nested classes instead of normal ones, but I can't see why it should be bad to do like this... Is there any problem with my code (the 2nd one)? Should I follow what the book says because my implementation of Comparator is wrong? (Note: the code compiles and runs without problem, with the expected output in both cases)

[cs012 Harrison 2.7, cs011 Lennon 3.1, cs021 McCartney 3.4, cs022 Starr 3.7]

Please help :D Thanks in advance.

Answer 1

You can implement a Comparator as a static nested class of the one being compared, if you are in control of that class (and if it is a class rather than an interface). It is not at all uncommon, however, that you want to compare instances of a class that you do not control, according to an order that the target class does not support natively (whether by being Comparable or by providing a Comparator class). In that case you must create your own, separate Comparator.

Even when you control everything, it is somewhat a matter of taste whether to implement Comparators as top-level classes. I'm not sure why you call it "uncomfortable"; myself, I usually prefer to avoid nested classes when I can. Note, too, that whether you nest or not, the Comparator implementation class will be compiled to a separate class file.

Answer 2

There's no real reason why you shouldn't actually implement both Comparable and Comparator.

class Student implements Comparable<Student>, Comparator<Student> {

    private final String id;
    private final String name;
    private final Double cgpa;

    public String getName() {
        return name;
    }

    public String getId() {
        return id;
    }

    public Double getCgpa() {
        return cgpa;
    }

    public Student(String studentId, String studentName, double studentCGPA) {
        id = studentId;
        name = studentName;
        cgpa = studentCGPA;
    }

    @Override
    public String toString() {
        return id + " " + name + " " + cgpa;
    }

    @Override
    public int compareTo(Student that) {
        return this.id.compareTo(that.id);
    }

    @Override
    public int compare(Student o1, Student o2) {
        return o1.getCgpa().compareTo(o2.getCgpa());
    }
}

It is often more appropriate, however, to implement only Comparable and use other methods (such as inner classes or anonymous classes) to choose different orders.

    class ByCgpa implements Comparator<Student> {

        @Override
        public int compare(Student o1, Student o2) {
            return o1.getCgpa().compareTo(o2.getCgpa());
        }

    }

    Collections.sort(list, new ByCgpa());

static void sortByCgpaAndName(Collection<Student> students) {
    Collections.sort(students, new Comparator<Student> () {

        @Override
        public int compare(Student o1, Student o2) {
            int byCgpa = o1.getCgpa().compareTo(o2.getCgpa());
            return byCgpa != 0 ? byCgpa : o1.name.compareTo(o2.name);
        }
    });
}

See here for further discussion.