Good way to change sort criteria for java class

Question

I have a class representing a column. It has a comparator which looks something like this:

class Column
{
    int xposition;
    int usage;

    @Override
    public int compare(Object arg0, Object arg1) 
    {
         // sort logic
    }
}

I have a TreeSet of Columns. I want to sort the TreeSet first by x-position, and then at some point by usage.

What I tried is to create a superclass, such as Column2, that extends Column and has a different compare method. However that makes converting from Column to Column2 (or visa versa) very ugly. I also thought of a flag in the Column that indicates how to do the sort, but that would mean modifying all the objects in order to change the sort criteria.

Is there any better way to do this?

First things first: `Column` should implement `Comparator`, so that you `compare` method takes two `Something`s as parameters, not just plain `Object`. But is `Column` really a `Comparator`, or do you intend it to implement `Comparable`? — Andy Turner, Jan 16 '17 at 17:16
@AndyTurner If the sorting logic alternates regularly, that might not be the best idea. — shmosel, Jan 16 '17 at 17:18
@shmosel well it's got to implement or extend *something*, or that `@Override` is going to prevent compilation. And presumably that's the `compare` method from `Comparator`. — Andy Turner, Jan 16 '17 at 17:19
@AndyTurner Although the given code is invalid: This is exactly what led to the problem, as far as I understood. **The class itself** should **NOT** implement `Comparator`. Instead, there should be *multiple* classes that are *only* different implementations of the `Comparator` interface - namely, different implementations of `Comparator` — Marco13, Jan 16 '17 at 17:19
@AndyTurner Or the method can be removed in favor of custom comparators. — shmosel, Jan 16 '17 at 17:19

Sean Bright · Accepted Answer · 2017-01-17T01:22:08.287

I would have the comparison logic in a set of external Comparators to represent the different sorting cases you have, and then create a new TreeSet when you want to change the sort:

class Column
{
    int xposition;
    int usage;

    public static final Comparator<Column> SortByX = new Comparator<Column>() {
        @Override
        public int compare(Column c1, Column c2)
        {
            return Integer.compare(c1.xposition, c2.xposition);
        }
    };

    public static final Comparator<Column> SortByUsage = new Comparator<Column>() {
        @Override
        public int compare(Column c1, Column c2)
        {
            return Integer.compare(c1.usage, c2.usage);
        }
    };
}

TreeSet<Column> cols = new TreeSet<>(Column.SortByX);

Then, to change the sort:

TreeSet<Column> updated = new TreeSet<>(Column.SortByUsage);
updated.addAll(cols);
cols = updated;

With appropriate synchronization if this is happening in a multi-threaded environment.

Whatever you do, do not change the behavior of an object's Comparator using mutable state. If you do, you could easily "lose track" of an object after it has been put into a collection like TreeSet.

Yes, this is exactly what I needed. I was unaware that I could pass a Comparator to the TreeSet constructor. — Fred Andrews, Jan 17 '17 at 00:48

score 0 · Answer 2 · edited May 23 '17 at 10:29

Strategy pattern

What you want to achieve seems a classic use case for the Strategy pattern (e.g. Real World Example of the Strategy Pattern)

In essence, you want to package up this comparison function into something that you can put in a separate field for your column class - a plain class with that single function that implements Comparable would work. Then, your column would just delegate the call to whatever comparator is stored in that field.

score 0 · Answer 3 · answered Jan 16 '17 at 17:22

0

This is exact use case of Guava's ComparisionChain:

Example taken from here :

public int compareTo(Foo that) {
     return ComparisonChain.start()
     .compare(this.xposition, that.xposition)
     .compare(this.usage, that.usage)
     .result();
}

answered Jan 16 '17 at 17:22

rkosegi

14,165
5
50
83

score 0 · Answer 4 · answered Jan 16 '17 at 17:54

Like Sean Bright answer, I'd use external Comparator and if you are working with Java 8, you can do it pretty easily:

public static final Comparator<Foobar> NAME_THEN_AGE = 
  Comparators.comparing(Foobar::getName, String.CASE_INSENSITIVE_ORDER)
             .thenComparing(Foobar::getAge)
;

....
TreeSet<Foobar> foobar = new TreeSet<>(NAME_THEN_AGE);

However, better remaindered that not, it is generally a bad idea to override Comparable on a child class - perhaps it should be final on the parent or one should create a protected compareTo0(A) doing the common work (to avoid comparing object by their parent class).

There are reason for that, and one such is the following (from the Javadoc of Comparable.compareTo):

The implementor must ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y. (This implies that x.compareTo(y) must throw an exception iff y.compareTo(x) throws an exception.)

Let's say you have class B and C extending A and A implements Comparable<A>:

class A implements Comparable<A> {
  @Override
  public int compareTo(A other) {return ...;}
}
class B extends A {
  @Override
  public int compareTo(A other) {return compareToAsB(((B)other));}
}
class C extends A {
  @Override
  public int compareTo(A other) {return compareToAsC(((C)other));}
}

It does not really matter what A::compareTo returns. Neither what compareToAsB and compareToAsC does.

The problem is however here:

A a = ...;
B b = ...;
C c = ...;

a.compareTo(b); // ok
a.compareTo(c); // ok
b.compareTo(a); // ko ClassCastException
b.compareTo(c); // ko ClassCastException
c.compareTo(a); // ko ClassCastException
c.compareTo(b); // ko ClassCastException

As quoted in the javadoc, a.compareTo(b) should throw a ClassCastException.

Also, there are part in the Java code (Collections.sort) where it is important to ensure that sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y.

Good way to change sort criteria for java class

4 Answers4

Strategy pattern