Java: Reference from a non-static comparator to it's outer class field

Question

I need to sort lists of objects with a non-static comparator that uses a value from it's outer object field.

class A {
    public int x;
    public int y;
    public int z;

    public Comparator<A> scoreComparator = new Comparator<A>() {
        public compare(A o1, A o2) {
            // System.out.println("this: " + this);
            return (int) (x * o1.x - x * o2.x);
        }
    }

    public A(int _x, int _y, int _z) {
        x = _x;
        y = _y;
        z = _z;
    }
}

A var_1 = new A(1, 2, 3);
A var_2 = new A(5, 6, 7);
List<A> list = getMyListFromSomewhere();

// the following will produce different ordering
Collections.sort(list, var_1.scoreComparator);
Collections.sort(list, var_2.scoreComparator);

But for some reason this does not work properly. When I uncomment the println line in the comparator, it shows that the references are to A objects, but they are different within one sort() call, therefore the value of "x" is different. What am I doing wrong here?

Answer 1

Can you explain why do you need the Comparatorto be non-static? Why not just the following?

    static class MyComparator implements Comparator {
        public compare(A o1, A o2) {
            // System.out.println("this: " + this);
            return o1.x - o2.x;
        }
    }

    public Comparator scoreComparator = new MyComparator();

Answer 2

That depends on what you want to achieve. The code above doesn't work because you use different values of x when you create your A instances.

Each time, you create an instance of A, you also create an instance of the comparator which is tied to the instance of A. That means the x in the compare() method is either o1.x or o2.x.

I suggest to create a new class that implements the comparator and which has a field x to make it independent of A:

public class ScoreComparator implements new Comparator<A>() {
    private int x;
    public ScoreComparator(int x) { this.x = x; }
    public compare(A o1, A o2) {
        // System.out.println("this: " + this);
        return (int) (x * o1.x - x * o2.x);
    }
}

Answer 3

Let's see first what the scoreComparator does. The line

(int) (x * o1.x - x * o2.x)

could be also written as

(int) x * (o1.x - o2.x)

which means the the sign of x -- positive or negative inverts comparation result will revert the ordering in the sorting list.

Casting to int is added to ensure integer overflow if values of x and o1.x or x and o2.x are too big. Again, the sign of x will just revert the ordering.

Since both var_1 and var_2 have positive values for the field x we can conclude that the second scenario causes integer overflow and different ordering. var_1.x equals to 1, and var_2.x equals to 5 which makes integer overflowing five times more probable in the latter case.

Answer 4

I'm not 100% sure what you want to achieve by this design, but this is a very bad design. If you want a non-static comparator inside the same class type, try going for compareTo rather than compare. Otherwise, put the compare method in a separate class as @Aaron suggested.