CODEWITHSUNDEEP
javacomparecomparator
Joseph Jones
Joseph Jones

Reputation: 9

Java - What would be the better method for a compare method. - 2 examples

I have a few compare method in a class and im not sure which way i should do them

First option i came up

public static Comparator<Student> nameCompare(String str) {
    if (str.equals("ASC")) {
        return new Comparator<Student>() {
            @Override
            public int compare(Student o1, Student o2) {
                return o2.getName().compareTo(o1.getName());
            }
        };
    } else {
        return new Comparator<Student>() {
            @Override
            public int compare(Student o1, Student o2) {
                return o1.getName().compareTo(o2.getName());
            }
        };
    }
}

or would it be better to use this

public static Comparator<Student> nameCompare(String str) {
    return new Comparator<Student>() {
        @Override
        public int compare(Student o1, Student o2) {
            if (str.equals("ASC")) { // here would be the str purple and underline 
                return o2.getName().compareTo(o1.getName());
            } else {
                return o1.getName().compareTo(o2.getName());
            }
        }
    };
}

what option would be better or to say it differently what would be a more proper way to code because the second is shorter but the str would be purple and underline because it is a local variable from the outer methode would it be bad to to it that way?

Upvotes: 0

Views: 118

Answers (3)

Ane De Jong
Ane De Jong

Reputation: 106

TL;DR

public static enum SortDirection { ASC, DESC };

public static Comparator<Student> nameCompare(SortDirection sortDirection)
{
    return switch (sortDirection) {
        case ASC -> Comparator.comparing(Student::getName);
        case DESC -> Comparator.comparing(Student::getName).reversed();
    };
}

enum because validation

First your nameCompare method should validate its parameter. You want to support two cases, ascending and descending sort order. So to reduce the possibility of error use an enum with only those two values rather than a String with virtually infinite values. Then the only validation you need to do explicitly is a null check.

switch expression

If you’re on Java 15 or later, this task is great for a switch expression as shown. If sortDirection is null, the switch expression throws a NullPointerException, so even the null check is built in. From the Java Language Updates:

However, for enum switch expressions that cover all known constants, the compiler inserts an implicit default clause.

If not yet on Java 15 or in a class that doesn’t allow switch expressions yet, either an old-fashioned switch statement or the if-else construct that you used in the question will do. You may even consider the ? : ternary operator. For a null check you may use

    Objects.requireNonNull(sortDirection);

Comparator.comparing()

Since Java 8 use Comparator.comparing() and related methods for constructing your comparators. It’s not only a lot terser, it also reduces the possibility of errors. Writing comparators the old-fashioned way that you used is very error-prone. I am talking from having seen them wrong in production code many times. For your own comparators, have you accidentally swapped the ascending and the descending one? With the terser way of building them, it’s harder to come into doubt about this.

Your concerns

You wrote:

… what would be a more proper way to code because the second is shorter but the str would be purple and underline

This isn’t a strong argument either way. While I advocate terse code above, short is not always better. Readability is what counts, not brevity. The purple underline is what IntelliJ IDEA (and possibly other IDEs, I don’T know) puts there to mark that inside the comparator we’re using a variable from outside the comparator. It’s perfectly valid to do (as long as the variable is effectively final). If you don’t like to see the purple line, I am pretty sure it can be turned off in the settings.

Tutorial link

Switch Expressions from Java Language Updates on Java 17

Upvotes: 3

Juan C Nuno
Juan C Nuno

Reputation: 563

Use the Comparator API directly:

    public static void main(String[] args) {
        var students = new ArrayList(List.of(new Student("A"), new Student("B")));
        var ascComparator = Comparator.comparing(Student::getName);
        var descComparator = Comparator.comparing(Student::getName).reversed();

        students.sort(ascComparator);
        System.out.println(students);

        students.sort(descComparator);
        System.out.println(students);
    }

Upvotes: 1

user25842772
user25842772

Reputation:

Option #3, separate them into two static methods

public static Comparator<Student> ascendingNameComparator() {
    return new Comparator<Student>() {
        @Override
        public int compare(Student o1, Student o2) {
               return o1.getName().compareTo(o2.getName());
        }
    };
}

        
public static Comparator<Student> descendingNameComparator() {
    return new Comparator<Student>() {
        @Override
        public int compare(Student o1, Student o2) {
               return o2.getName().compareTo(o1.getName());
        }
    };
}

This solution will always be better than having to check the input in runtime and hoping that whoever calls this method actually understands what it expects as input

Upvotes: 0

Related Questions