error in arraylist sorting using collections.sort()

Question

I am writing a code on fractional knapsack. But, the sorting algorithm is not working well. And hence my code is not working correctly.

This is my code:

import java.util.*;
class Item {
    int profit, weight;
    float ppw;
    //ppw is profit per weight
    public Item(int profit, int weight) {
        this.profit = profit;
        this.weight = weight;
        this.ppw= (float)profit / (float)weight;
    }
}

class SortbyPPW implements Comparator\<Item\> {
    public int compare(Item o1, Item o2) {
        return (int) (o2.ppw) - (int) (o1.ppw);  
    }
}

public class FractionalKnapSack {
    public static void main(String args\[\]) {
    Scanner sc = new Scanner(System.in);
    System.out.print("Enter the number of elements: ");
    int n = sc.nextInt();
    System.out.print("Enter maximum capacity: ");
    int capacity = sc.nextInt();
    System.out.println("\\nEnter the items with their profit and weight separated by a space");

    ArrayList<Item> items = new ArrayList<Item>();
    for (int i = 0; i < n; i++) {
        int profit = sc.nextInt();
        int weight = sc.nextInt();
        items.add(new Item(profit, weight));
    }

    System.out.println("\n");
    for (Item item : items)
        System.out.print(item.ppw+" ");
    System.out.println("\n");


    System.out.println(Arrays.toString(items.toArray()));
    double[] maxValue = getMaxValue(items, capacity);
    System.out.println("\nMaximum profit we can obtain = " + maxValue[0]);
    System.out.println("Capacity Used = " + (capacity-maxValue[1]));
    System.out.println("\n");
    System.out.println(Arrays.toString(items.toArray()));

    System.out.println("\n");
    for (Item item : items)
        System.out.print(item.ppw+" ");
    System.out.println("\n");
}

public static double[] getMaxValue(ArrayList<Item> items, int capacity) {
    double maxValue = 0;
    Collections.sort(items, new SortbyPPW());

    for (Item item : items) {
        int currWeight = item.weight;
        int currValue = item.profit;

        if (capacity - currWeight >= 0) {
            capacity -= currWeight;
            maxValue += currValue;
        } else {
            double fraction = ((double) capacity / (double) currWeight);
            maxValue += (currValue * fraction);
            capacity = (int) (capacity - (currWeight * fraction));
            break;
        }
    }
    double[] arr1={maxValue, (double)capacity} ;
    return arr1;
}

}

I tried using simple array instead of arraylist and it worked correctly but my teacher wants to code to have collections.sort() and comparator class implemented. This is the input 8 40 20 10 30 15 18 5 25 12 14 6 24 8 15 7 10 3

Expected Output 103.91 Actual Output 100.0

Input and Output on my laptop

But, my code here got close only upto 100 and thus it's not correct completely.

Answer 1

I tried using simple array instead of arraylist and it worked correctly

I find that hard to believe. The Comparator you created is deficient, whether used with an array or with a List.

You are sorting based on Item.ppw, which is a float. There's nothing inherently wrong with that, but consider your specific implementation:

        return (int) (o2.ppw) - (int) (o1.ppw);  

If o1.ppw and o2.ppw are different, but (int) o1.ppw and (int) o2.ppw are the same, then the wrong result (0) will be returned.

I'm inclined to guess that you tried to adapt a more subtly flawed yet relatively common implementation for int-based comparisons. Using a difference of two ints as the return value of a comparator is more right than your code, but it will still produce the wrong answer if the subtraction overflows.

Don't try to be so clever. A Comparator of this general form is perfectly good for comparing objects by any single field of primitive type:

class CompareByPPW implements Comparator<Item> {
    public int compare(Item o1, Item o2) {
        if (o1.ppw < o2.ppw) {
            return -1;
        } else if (o2.ppw < o1.ppw) {
            return 1;
        } else {
            return 0;
        }
    }
}