Comparable Java Ordering

Question

import java.util.*;
public class TestPerson{
    public static void main(String[] args){
        person Albert = new Person(1);
        person James = new Person(2);
        person Elizabeth = new Person(3);
        person [] personArray = new Person[3];
        personArray[0] = Albert;
        personArray[1] = James;
        personArray[2] = Elizabeth;
        Arrays.sort(personArray);
        System.out.println(personArray[0].number);
        System.out.println(personArray[1].number);
        System.out.println(personArray[2].number);
    }
}

---

public class Person implements Comparable{
    int number;
    public Person(int number){
        this.number = number;
    }
    public int compareTo(Object o){
        if(!(o instanceof person)){
            System.out.println("error");
            System.exit(1);
            person newObject = (person) o;
            if (this.number > newObject.number){
                return 1;
            }
            else if(this.number == newObject.number){
                return 0;
            }
        }
        return -1;
    }
}

I'm confused as to why the output is sorted backwards: 3,2,1. My compareTo method returns a positive number when this.number>number, so shouldn't that ensure that the numbers are sorted in order -- from smallest to greatest? Thanks

Answer 1

Comparator.comparingInt

As others said, your compareTo implementation is incorrect. In addition, you should use Java Generics rather than a raw type with your implements Comparable. And, by the way, System.exit(1); is an extreme way to handle a sorting issue.

Furthermore, the Comparator class provides a convenient implementation for comparing int values, so you need write no code at all for your comparison. Call Comparator.comparingInt to access a Comparator object for comparing int values.

Define your Comparator up front once, as a static constant. Because it is a constant, name in all uppercase per Java conventions.

We can most briefly define our Comparator object by using a method reference to access the field being compared. So we need to add an accessor ("getter") method to your class.

To make your class a bit more realistic, let's rename number to id. And add another member field, the person’s name.

In doing all that, your code:

public class Person implements Comparable{
    int number;
    public Person(int number){
        this.number = number;
    }
    public int compareTo(Object o){
        if(!(o instanceof person)){
            System.out.println("error");
            System.exit(1);
            person newObject = (person) o;
            if (this.number > newObject.number){
                return 1;
            }
            else if(this.number == newObject.number){
                return 0;
            }
        }
        return -1;
    }
}

… becomes:

public class Person implements Comparable < Person >
{
    // Statics.
    static final Comparator < Person > COMPARATOR = Comparator.comparingInt( Person :: getNumber );

    // Member fields.
    int number;
    String name;

    // Constructors.
    public Person ( final int number , final String name )
    {
        this.number = number;
        this.name = name;
    }

    // Accessors. 
    public int getNumber ( )
    {
        return number;
    }

    // Implement `Comparable` interface.
    @Override
    public int compareTo ( final Person other )
    {
        return Person.COMPARATOR.compare( this , other );
    }
}

compareTo logic consistent with equals & hashCode

In addition, your class’ implementation of equals & hashCode methods inherited from Object should always have logic consistent with your compareTo. So we need to provide overrides of those methods that look at the id field.

package work.basil.example.comparing;

import java.util.Objects;
import java.util.TreeSet;
import java.util.Comparator;
import java.util.List;

public class Person implements Comparable < Person >
{
    // Privates.
    private static final Comparator < Person > COMPARATOR = Comparator.comparingInt( Person :: getId );

    // Member fields.
    int id;
    String name;

    // Constructors.
    public Person ( final int id , final String name )
    {
        this.id = id;
        this.name = name;
    }

    // Accessors.
    public int getId ( )
    {
        return id;
    }

    // Implement `Comparable` interface.
    @Override
    public int compareTo ( final Person other )
    {
        return Person.COMPARATOR.compare( this , other );
    }

    // `Object` overrides.

    @Override
    public boolean equals ( final Object o )
    {
        if ( this == o ) return true;
        if ( o == null || getClass( ) != o.getClass( ) ) return false;
        final Person person = ( Person ) o;
        return id == person.id;
    }

    @Override
    public int hashCode ( )
    {
        return Objects.hashCode( id );
    }
}

Record

If the main purpose if this Person class is to transparently communicate shallowly-immutable data, make it a record. As a side-effect, this happens to reduce our code, as the compiler implicitly creates a record class’ constructor, getters, equals & hashCode, and toString.

But we still need to provide our own override of equals & hashCode because our logic looks only at a single member field, id. In contrast, a record’s default implementation looks at each and every member field which would be inconsistent with our compareTo logic.

public record Person( int id , String name ) implements Comparable < Person >
{
    // Privates.
    private static final Comparator < Person > COMPARATOR = Comparator.comparingInt( Person :: id );

    // Implement `Comparable` interface.
    @Override
    public int compareTo ( final Person other )
    {
        return Person.COMPARATOR.compare( this , other );
    }

    // `Object` overrides.

    @Override
    public boolean equals ( final Object o )
    {
        if ( this == o ) return true;
        if ( o == null || getClass( ) != o.getClass( ) ) return false;
        final Person person = ( Person ) o;
        return id == person.id;
    }

    @Override
    public int hashCode ( )
    {
        return Objects.hashCode( id );
    }
}

Run this code

Some code to exercise this class. We use the TreeSet class here, an implementation of the SequencedSet/NavigableSet/SortedSet interfaces. This keeps our sample data objects in order of our compareTo method.

public static void main ( String[] args )
{
    System.out.println(
            new TreeSet < Person >(
                    List.of(
                            new Person( 99 , "Carol" ) , new Person( 7 , "Alice" ) , new Person( 42 , "Bob" )
                    )
            )
    );
}

[Person[id=7, name=Alice], Person[id=42, name=Bob], Person[id=99, name=Carol]]

Answer 2

You only need to change your compareTo method to the following, so it can return an error in case the passed is not an instance of person, otherwise it will cast the passed object and compare it:

 public int compareTo(Object o){
    if(!(o instanceof person)){
        System.out.println("error");
        System.exit(1);
    }

    person newObject = (person) o;
    if (this.number > newObject.number){
       return 1;
    }
    else if(this.number == newObject.number){
       return 0;
    }
    return -1;
}

Answer 3

You have an error in your compareTo method which always returns -1. However you can simplify your compareTo method quite a bit by implementing Comparable<Person> rather than Comparable:

class Person implements Comparable<Person> {
    public int compareTo(Person other) {
        return this.number - other.number;
    }
}