Sort based on Factors using Custom Comparator in Java-CodePudding

Trying to sort the array based on the number of factors using a custom comparator function, but the output remains same as input. Sorting isnt working

    public void solve(){
        Integer[] A = new Integer[]{3,9,4,6,12,10,7}; 

        System.out.println(Arrays.toString(A));
        Arrays.sort(A, (A1, B) -> {
            int fact_a = calcFactors(A1);
            int fact_b = calcFactors(B);
            if(fact_a < fact_b) return 1;
            if(fact_a==fact_b){
                if(A1 < B) return 1;
            }
            return 0;
        });
        System.out.println(Arrays.toString(A));
    }

    public int calcFactors(int A){
        int count = 0;
        int sqrt = (int)Math.sqrt(A);
        for(int i =1;i<=sqrt;i  ){
            if(A%i == 0) count =2;
        }
        if(sqrt*sqrt == A) count--;
        return count;
    }

CodePudding user response：

It looks like your comparator only checks for the case that a < b. Properly implemented comparators need to return not only 1 when a > b, but also 0 when a == b and -1 when a < b.

You could expand your if logic to cover those cases, but fortunately, there's an easier way - Integer.compare():

int result = Integer.compare(fact_a, fact_b);
if (result == 0) {
  result = Integer.compare(A1, B);
}
return result;

Integer.compare() handles most of the logic for you; the only thing you need to do is check if the result of the factor comparison is 'equal', and if so, apply the fallback comparison of the numbers themselves

(note: for the sake of this answer I'm assuming that calcFactors is implemented properly...)

CodePudding user response：

Your comparator should return -1 if A1 is intended to come before B in the sort order, or 1 if A1 is intended to come after B. Returning 0 should only happen if A1 and B are equal in terms of the sorting. But you have returned 0 in a lot of cases where A1 is intended to come after B.

I think you intended your logic to be something like this.

if(fact_a < fact_b){
     return -1;
}
else if(fact_a == fact_b){
    if(A1 < B) {
       return -1;
    }
    else if(A1 == B) {
       return 0;
    }
    else {
       return 1;
    }
}
else {
    return 1;
}

A simpler way of writing this is found in Kevin K's answer, which I have just upvoted. But this shows you, longhand, what the logic needs to be.

CodePudding user response：

I think you should check your lambda expression, there should be some cases in which the return value is -1. In your code, the return value is either 0 or 1, which means the first argument is always equal or greater than the second, so the order will not change.