Basic Java bank account

Question

I'm having a bit of trouble setting up a basic bank account in Java. I'm trying to create a transfer method that I can call to transfer funds from one account to another. The problem is that the method I have created has an unnecessary step where I have to declare the name of the object twice. To execute the transfer method, my current code requires me to write michael.acc.transfer(michael, max); if I want to transfer funds from michael to max. Any ideas on how I can simplify this a bit?

For example: michael.acc.transfer(max);

Here is the class I created with the methods that I can execute to an account:

public class Account {

static double balance;
String accountId;
static int nextId = 0;
static final int ROUTING_NUMBER = 12345;
String bankName;

{
    if (ROUTING_NUMBER == 12345) {
        bankName = "USA Bank";
    }
    else {
        bankName = "Other bank";
    }
}

public void deposit(double amount) {
    balance = balance + amount;
}


public void withdraw(double amount) {
    balance = balance - amount;
}


public void transfer (Customer c1, Customer c2) {
    double transferAmount;
    int routingNumber;
    {
        Scanner input = new Scanner(System.in);
        System.out.println("Please enter transfer amount: ");
        transferAmount = input.nextDouble();

        System.out.println("Please enter recipient's routing number: ");
        routingNumber = input.nextInt();

        if (routingNumber == ROUTING_NUMBER) {
            System.out.println("Your funds will transfer instantly, you and your recipient share the same bank!");
            System.out.println("Bank name: " + bankName);
        } else {
            System.out.println("Your funds will transfer in 2-3 business days.");
        }
    }
    c1.customerBalance -= transferAmount;
    c2.customerBalance += transferAmount;

}
public static String getNextId() {
    return "ACCT #" + nextId++;
}

The next class is the one I created with methods to create the customer:

public class Customer {

public String firstName;
public String lastName;
public Account acc;
public double customerBalance = acc.balance;
int defaultBalance = 100;


public Customer(String firstName, String lastName) {
    this.firstName = firstName;
    this.lastName = lastName;
 }


public Customer() {
    firstName = "John";
    lastName = "Doe";
 }


public void addAccount(double initialBalance) {
    acc = new Account();
    acc.accountId = "ACCT ID: " + Account.getNextId();
    customerBalance = initialBalance;
 }


public void addAccount() {
    addAccount(100);
 }

}

The following are the two accounts that I created to transfer funds from one to the other:

public class Bank {

public static void main(String[] args) {

    Customer max = new Customer("Max", "Doe");
    max.addAccount(1500);

    Customer michael = new Customer("Michael", "Smith");
    michael.addAccount(3000);

 }
}

Answer 1

I don't see any reason why you have to pass the two customers to you transfer method. But first, there are some main remarks :

• Do you want to do a transfer from Customer or Account ? Usually I transfer from different accounts not customers. So let's assume that what you want is a method to transfer a given amount of money from one account to another.
• You cannot assume that you will have a console when transferring. How can someone reuse this code for a web app ? Please, use a parameter to receive the amount to transfer. And in the meantime, don't ask for the recipient's routing number, because it will be into the recipient account.

So you will have a method like Account.transfer(double amount, Account recipient)

@Test
public void transfer_must_move_amount() {
    Account michael = new Account();
    Account max = new Account();

    michael.transfer(15, max);

    assertThat(max.getBalance()).isEqualTo(15);
}

Your code just have to be refactored a little :

public void transfer(double amount, Account recipient) {
        double transferAmount = amount;
        int routingNumber = recipient.ROUTING_NUMBER;
            if (routingNumber == ROUTING_NUMBER) {
                System.out.println("Your funds will transfer instantly, you and your recipient share the same bank!");
                System.out.println("Bank name: " + bankName);
            } else {
                System.out.println("Your funds will transfer in 2-3 business days.");
            }
        this.balance -= transferAmount;
        recipient.balance += transferAmount;
    }

This test will fail with Expected: 15.0, Actual: 0.0. This is because you balance is static and static members belong to the class instead of one Account. Remove it and everything will be okay.

But there is much more than can be improved on your code.

---

Protect the Account#balance property form the outside; make it private and initialize it to 0 to be sure.

By doing that your Customer will be red because acc.balance is not avlid anymore. This a good thing because by copying the value, if you change the balance in your Account the customer one will not change anymore. Remove it and replace it by a getBalance() method that will return the balance from your Account.

Since you have one addAccount method, I suppose that a customer may have many accounts, so replace the Account acc field with a collection of Account. The getBalance will returns the sum of all accounts.

Back to the Account where your two deposit and withdraw methods are unused. You may use them via your transfer method. This may safe you later if some Account have some fees on those operations. You just have to override those methods and the transfer method will not be touched.

The only things that we have not touched is the ROUTING_NUMBER. Usually this information is part of your account number. So it may be the good time to add a AccountNumber class. This one is a value object that enforce some rules and can be used to verify if two accounts are in the same bank.

-- I have ignored the nextId and bankName from this review because they are out of your "transfer" scope.

https://github.com/gervaisb/stackexchange-codereview/tree/q183675