Im trying to sort an array list with the compareTo method

Question

I am trying to sort Bank accounts where it first sorts by Name alphabetically, and second by the amount in the account from most to least. Unfortunately the CompareTo method doesn't seem to be working correctly, the only part that works is the second part where it sorts by the amount.

BankAccount class

/**
 * A bank account has a balance, the name of the account holder,
 * and an account number. The balance can be changed by deposits
 * and withdrawals.
 */
public class BankAccount implements Comparable<BankAccount> {

/**
 * Constructs a bank account with a zero balance.
 * @param name the name of the account holder
 */
public BankAccount(String name) {
   this.name = name;
   balance = 0;
   accountNo = ++lastAccountNo;
}

/**
 * Constructs a bank account with a given balance.
 * @param initialBalance the initial balance
 * @param name the name of the account holder
 */
public BankAccount(String name, double initialBalance) { 
    this.name = name;
    balance = initialBalance;
    accountNo = ++lastAccountNo;
}

/**
 * Deposits money into the bank account.
 * @param amount the amount to deposit
 */
public void deposit(double amount) {
    double newBalance = balance + amount;
    balance = newBalance;
}

/**
 * Withdraws money from the bank account.
 * @param amount the amount to withdraw
 */
public void withdraw(double amount) {
    double newBalance = balance - amount;
    balance = newBalance;
}

/**
 * Gets the current balance of the bank account.
 * @return the current balance
 */
public double getBalance() {
    return balance;
}

/**
 * Gets the name of the account holder.
 * @returns the name of the account holder
 */
public String getName() {
    return name;
}

/**
 * Gets the account number of the account.
 * @returns the account number of the account
 */
public int getAccountNo() {
    return accountNo;
}

/**
 * Returns a String representation of the BankAccount. The format
 * is "name: accountNo balance"
 * @returns a String representation of the BankAccount.
 */
public String toString() {
    return name + ": AccountNo:" + accountNo + " balance:" + balance;
}

private double balance;
private String name;
private int accountNo;
private static int lastAccountNo=0;

public int compareTo(BankAccount b) {

    if(this.name.compareTo(b.name) == 0 && this.balance > b.balance) return 0;
    else if(this.name.compareTo(b.name) < 0 && this.balance < b.balance) return 1;
    else if(this.name.compareTo(b.name) > 0 && this.balance == b.balance) return -1;

    //else if(this.name.compareTo(b.name) == 0) return 0;
    //else if(this.name.compareTo(b.name) < 0) return 1;
    //else if(this.name.compareTo(b.name) > 0) return -1;
    else if(this.balance == b.balance) return 0;
    else if(this.balance < b.balance) return 1;
    else if(this.balance > b.balance) return -1;
    else return 0;
}
}

BankAccountTester

import java.util.ArrayList;
import java.util.Collections;

class BankAccountTester {
    public static void main(String args[]) {
        ArrayList<BankAccount> accounts = new ArrayList<BankAccount>();
        accounts.add(new BankAccount("Louis Oliphant", 100.0));
        accounts.add(new BankAccount("Louis Oliphant", 100.10));
        accounts.add(new BankAccount("Louis Oliphant", 100.0));
        accounts.add(new BankAccount("Jane Doe", 100.0));
        accounts.add(new BankAccount("Jane Doe", 99.0));
        accounts.add(new BankAccount("Jane Doe", 100.0));
        System.out.println("*****Unsorted******");
        for (BankAccount b : accounts) {
            System.out.println(b);
        }
        System.out.println();
        Collections.sort(accounts);
        System.out.println("******Sorted******");
        for (BankAccount b : accounts) {
            System.out.println(b);
        }
    }
}

Dont you need to return 0 when both balance and name of two objects are same? You are returning -1 — Muhammad Tauseef, Mar 18 '14 at 01:03
Do the comparison in 2 steps. Firstly, compare names, then compare the balances. For each possible outcome of name comparison (equals, less than, greater than) there is an equivalent number of balance comparisons. If you look at your code again, you don't cover all possibilities. As an aside, you wouldn't use a double to represent money because of rounding errors. It's better to use BigDecimal. Write the code for the case where names are equal plus the 3 possible balances. Then move to the next case. — Romski, Mar 18 '14 at 01:09
Use comparators. http://stackoverflow.com/questions/14154127/collections-sortlistt-comparator-super-t-method-example — goodcow, Mar 18 '14 at 01:35
I second the use of (external) `Comparator`s. You might want to have different order in different use cases. // And you might consider the question [Why not use Double or Float to represent currency?](http://stackoverflow.com/questions/3730019/) — Markus Mitterauer, Aug 26 '16 at 14:14

score 1 · Answer 1 · edited May 23 '17 at 12:33

You should reconsider your logic. If the names of both bank accounts are not equal, then the comparison of the amount is unnecessary, isn't it?

public int compare(BankAccount o1, BankAccount o2) {
    int compareName = o1.getName().compareTo(o2.getName());
    if (compareName == 0) {

        // When the balance of the current amount is greater than the
        // balance of the compared account, then the current accounts
        // "precedes" the compared account, and thus -1 is returned.
        // If the balance is less, than this account "follows" the
        // compared account, and 1 is preceded.
        // Otherwise, 0 is returned.
        return (0 - Double.compare(o1.getBalance(), o2.getBalance()));
    }
    else {
        return compareName;
    }
}

Or shorter:

public int compare(BankAccount o1, BankAccount o2) {
    if (o1.getName().compareTo(o2.getName()) < 0) { return -1; }
    else if (o1.getName().compareTo(o2.getName()) > 0) { return 1; }
    return (0 - Double.compare(o1.getBalance(), o2.getBalance()));
}

As others mentioned, I suggest using an apart Comparator<T> as argument to the sort() method, instead of implementing the compareTo() method of the interface Comparable. Implementing compareTo() defines the "natural" ordering of an object. Your ordering looks like an ordering specific to that case, and not the natural ordering. See Java : Comparable vs Comparator.

By the way, when working with monetary values, you should be using BigDecimals instead of doubles. See Double vs. BigDecimal?.

Note 1: The upper code block returns the lexicographical difference when the account names are equal, see String.compareTo(String), that is, they not always return -1, 0 or 1.

Note 2: When you are planning to also override the equals(Object) and hashCode() methods, keep in mind that it is strongly recommended, although not strictly required that x.compareTo(y) == 0 equals to x.equals(y), see Comparable<T>.compareTo(T object).

Why don't you just use `Double.compare(this.balance, b.balance)` to compare the balances? — Markus Mitterauer, Aug 26 '16 at 14:11
and, regarding `equals(Object)` it should be mentioned, that if overriding `equals(Object)` one is strictly required to also implement the `hashCode()` method. See also [What issues should be considered when overriding equals and hashCode in Java?](http://stackoverflow.com/questions/27581/) — Markus Mitterauer, Aug 26 '16 at 14:28

Im trying to sort an array list with the compareTo method

1 Answers1