My comparator's compare method isn't working

Question

I have a Customer object class which has some variables and is implementing a Comparator already regarding one of those variables. However I need to implement another Comparator for a different variable last_name.

Since I can't have 2 compareTo() methods in my Customer class I decided to make a Comparing class specifically for this here

public class CompareByLastName implements Comparator<Customer> {

    private List<Purchase> purchases;
    private List<Customer> customers;

    public CompareByLastName(List<Purchase> purchases, List<Customer> customers) {
        this.purchases = purchases;
        this.customers = customers;
    }

    /**
     * @param descending
     * @return will be a sorted, in ascending, or descending, array of customer's according to their authors.
     */
    public List<Purchase> sortByLastName(boolean descending){

        List<Purchase> return_List = new LinkedList<Purchase>();

        Collections.sort(customers);

        if(descending == true) {
            Collections.reverse(customers);
        }

        for(Customer customer : customers) {
            for(Purchase purchase_info : purchases) {
                if(customer.getId() == purchase_info.getCustomer_id()) {
                    return_List.add(purchase_info);
                }
            }
        }

        return return_List;
    }

    @Override
    public int compare(Customer customer_1, Customer customer_2) {

        int result = customer_1.getLastName().compareTo(customer_2.getLastName());

        if(result < 0) {
            return -1;
        }
        else if(result > 0) {
            return 1;
        }
        else {
            return 0;
        }
    }   
}

but once it hits Collections.sort(customers);

it doesn't activate the public int compare(Customer customer_1, Customer customer_2) below.

Frankly I don't know what its using as a comparator in the sort; does anyone know how to fix this problem and get it sorting by the last_name?

Oh and once it gets to return it some how manages to go from 100(0-99) items in purchases to 103(0-102) items in return list? No idea how that's happening.

fixed this part I had switch the for loops reading Purchase then going through the list of all the customers and finding matches instead of vise versa.

Any help is appreciated.

Thanks in advance.

Your comparator's `compare` method isn't working because you're not passing an instance of your comparator to `Collections.sort`. — Kevin Anderson, Jun 05 '20 at 05:21

score 1 · Accepted Answer · answered Jun 05 '20 at 05:25

1

You are not using your compare method.

    Collections.sort(customers);

The line above sort customers by their natural order (as defined by Customer.compareTo()), not according to your comparator. Instead you may do this:

    Collections.sort(customers, this);

Now your CompareByLastName will be use as comparator in the sorting.

A couple of asides:

Your design with the sort method in the comparator class is unconventional. It should work, though.
In your compare method you don’t need the if-else construct. The following much simpler implementation suffices:
```
   return customer_1.getLastName().compareTo(customer_2.getLastName());
```

answered Jun 05 '20 at 05:25

Ole V.V.

81,772
15
137
161

Thank you :D I tried a bunch of second params in .sort() it never occurred to me that the this keyword would work. – Mike_1234 Jun 05 '20 at 05:37
1

@Mike_1234 the reason, why the solution was not obvious to you, is that your entire design is counter-intuitive. You should not mix the `Comparator` implementation with the code performing the sort operation. In fact, you don’t need to implement the comparator yourself. Just `Collections.sort(customers, Comparator.comparing(Customer::getLastName));` would work. Or even simpler, `customers.sort(Comparator.comparing(Customer::getLastName));`. But actually, that’s not what you want. Your goal is to sort the *purchase list*, so you should do that &avoid the nested loop that performs n×m operations – Holger Jun 05 '20 at 15:12
2

@Mike_1234 i.e. `public static List sortPurchaseByCustomersLastName(List customers, List purchases, boolean descending) { List returnList = new ArrayList<>(purchases); Map – Holger Jun 05 '20 at 15:15

My comparator's compare method isn't working

1 Answers1