I have a class, Person, that has an attribute of ArrayList<Person> children;

I want to access and iterate over the children. Sometimes I need to sort the children list by their age, from oldest to youngest, but not always. When I do need to do it, I will need to do it in a number of places with a number of objects, so it makes sense that I don't do it with new Comparators each time.

Would this then be best accomplished by creating a compareTo(Person o) for the Person class, and then when I want to iterate over the sorted list of children in main(), I do something like:

The compareTo Method:

public class Person implements Comparable<Person> {
// This is in the Person class as a method.
@Override
public int compareTo(Person o) {
    if (this.age == o.getAge()) {
        return 0;   
    } else if (this.age < o.getAge()){
         return 1; // Because we want to go from largest to smallest.
    } else {
        return -1;
    }
  }
}

When I need to access each child of the person within the code and access from there:

Collections.sort(person.getChildren());
for (Person child: person.getChildren()){
    // Whatever needs to be done.
}

Is my understanding of this right?

CodePudding user response：

It doesn't matter what class the children field is a member of. What matters is the type of the children field itself.

Since the field is a list of Person, sorting it requires the Person class to implement Comparable interface, which is exactly what you did so you'd be fine. In fact, you'd be find even if you keep the ArrayList<Person> children field in some other class and not in the Person class itself.

CodePudding user response：

You can compare objects this way, but there are two important ways you can improve your approach.

1. Are people always sorted by age? More often, they're sorted by name. Sometimes they're sorted by height or weight. If there's not a unique inherently correct way to sort objects, it is usually better to have separate Comparators: Comparator<Person> BY_AGE, Comparator<Person> BY_NAME, and so on. You can make these constants (public static final) to avoid recreating them.

2. Your compareTo provides correct logic, but you can simplify it by using Integer.compareTo. Even better, you can write the comparator like this: Comparator.comparingInt(Person::getAge).

In total, the code would look like this:

public class Person {
  public static final Comparator<Person> BY_AGE = Comparator.comparingInt(Person::getAge);

  ...
}