CODEWITHSUNDEEP
javasortingcomparatorcomparable
Klausos Klausos
Klausos Klausos

Reputation: 16050

Sorting function provides incorrect result

I want to sort priorities[] in descending order i.e. ...2,1,0. When I execute the below-provided code, I receive unsorted array, e.g. 

18, 14, 15, 19, 23, 37, 35, 1, 8, 24, 26, 36

Why does this happen?

double[] priorities = new double[10];
for (int i = 0; i < 10; i++) 
    priorities[i] = Math.round(10*Math.random();
ArrayIndexComparator comparator = new ArrayIndexComparator(priorities,1);
Integer[] sortedPriorities = comparator.createIndexArray();
Arrays.sort(sortedPriorities, comparator);


public class ArrayIndexComparator implements Comparator<Integer>
{
    private final double[] array;
    private int sort;

    public ArrayIndexComparator(double[] array, int sort)
    {
        this.array = array;
        this.sort = sort;
    }

    public Integer[] createIndexArray()
    {
        Integer[] indexes = new Integer[array.length];
        for (int i = 0; i < array.length; i++)
        {
            indexes[i] = i;
        }
        return indexes;
    }

    @Override
    public int compare(Integer index1, Integer index2)
    {
        if (sort == 0)
            return Double.compare(array[index2],array[index1]); // ascending order 0,1,2,...
        else
            return Double.compare(array[index1],array[index2]); // descending order ...2,1,0
    }
}

Upvotes: 0

Views: 190

Answers (1)

Isaac
Isaac

Reputation: 16736

Using a debugger would show you why your comparator doesn't work. Seems like you overcomplicated it quite a bit. All the comparator is supposed to do is take two elements, compare them and return a result that would satisfy your ordering's requirement.

You're looking for a comparator that will effectively inverse the "natural" order of Doubles, so try something along the line of:

double[] priorities = new double[10];
for (int i = 0; i < priorities.length; i++) 
    priorities[i] = Math.round(10*Math.random());
Arrays.sort(priorities, new ArrayIndexComparator());

...
...

public class ArrayIndexComparator implements Comparator<Double> {
    @Override
    public int compare(Double d1, Double d2) {
        return -1*d1.compareTo(d2);
    }
}

(That's in a nutshell. You should really turn ArrayIndexComparator into a singleton, but that's beyond the point of this question)

If you're too lazy to do that, you can download Commons-Collections and use the built-in inverse comparator:

Arrays.sort(priorities, ComparatorUtils.reversedComparator(
    ComparatorUtils.naturalComparator()));

And then you don't even need your own custom comparator class.

Upvotes: 5

Related Questions