Bank simulation using ArrayList

Question

Problem Statement:

Design a BALANCE class with account number, balance and date of last updation. Consider a TRANSACTION class with account number, date of transaction, amount and transaction type. Check whether the amount is available or not in case of a withdrawal. Transaction object will make necessary updates in the BALANCE class.

Balance class

package balance;

import java.util.Date;
import java.util.Scanner;

public class Balance {
    private double balance;
    private Date date;
    private long accountNum;
    Scanner sc = new Scanner(System.in);

    public Balance(long aNo, double money, Date aDate) {
        accountNum = aNo;
        balance = money;
        date = (Date) aDate.clone();
        System.out.println("New account created with account no :" + accountNum);
        System.out.println("Opening balance rs. " + balance);
        System.out.println("Account created on " + date.toString());
    }

    public Balance(Balance b) {
        balance = b.balance;
        date = b.date;
        accountNum = b.accountNum;
    }

    public Balance() {

    }

    public long getAccountNum() {
        return accountNum;
    }

    public double getBalance() {
        return balance;
    }

    public void setBalance(double balance) {
        this.balance = balance;
    }

    public Date getDate() {
        return date;
    }

    public void setDate(Date date) {
        this.date = date;
    }

    public String toString() {
        return "A/C no.: " + accountNum + "\nCurrent balance: " + balance
                + "\nLast date of update: " + date;

    }

}

Bank class to operate on Balance class, containing an ArrayList of Balance objects

package balance;

import java.util.ArrayList;
import java.util.Iterator;

public class Bank {

    private ArrayList<Balance> balanceList;
    Iterator<Balance> itr;

    public Bank() {
        balanceList = new ArrayList<Balance>();
    }

    public void newAccount(Balance e) {
        balanceList.add(e);
    }

    public Balance searchAccount(long accountNum) {
        itr = balanceList.iterator();
        while (itr.hasNext()) {
            Balance b = new Balance(itr.next());
            if (b.getAccountNum() == accountNum)
                return b;
        }
        return null;
    }

    public long getBalanceListSize() {
        return balanceList.size();
    }
}

Transaction class to operate on a Bank

package balance;

import java.util.Date;
import java.util.Scanner;

public class Transaction {

    private String transactionType;
    private double amount;
    private long accountNum;
    private Date date;
    private Bank b;
    private Balance balance = new Balance();

    Scanner sc = new Scanner(System.in);

    public Transaction() {
        b = new Bank();
    }

    public void transaction(long accountNum, String transactionType,
            double amount) {
        this.accountNum= accountNum;
        this.transactionType = transactionType;
        this.amount = amount;
        date = new Date();
        operation();
    }

    private void operation() {

        if (transactionType.equalsIgnoreCase("Opening")) {
            if (amount < 0) {
                System.out.println("Opening balance cannot be less than zero.");
                return;
            }

            balance = new Balance(b.getBalanceListSize() + 1, amount, date);
            b.newAccount(balance);
        }

        else if (transactionType.equalsIgnoreCase("withdraw")) {
            balance = b.searchAccount(accountNum);
            if (balance == null) {
                System.out.println("Account not found");
                return;
            }
            if (balance.getBalance() < amount) {
                System.out.println("Insufficient Balance.");
                return;
            }
            System.out.println("Balance before transaction:");
            System.out.println(balance.toString());
            balance.setBalance(balance.getBalance() - amount);
            System.out.println("Balance after transaction:\n"
                    + balance.toString());
        }

        else if (transactionType.equalsIgnoreCase("deposit")) {
            balance = b.searchAccount(accountNum);
            if (balance == null) {
                System.out.println("Account not found");
                return;
            }
            System.out.println("Balance before transaction:");
            System.out.println(balance.toString());
            balance.setBalance(balance.getBalance() + amount);
            System.out.println("Balance after transaction:\n"
                    + balance.toString());
        }

        else if (transactionType.equalsIgnoreCase("showInfo")) {
            balance = b.searchAccount(accountNum);
            if (balance == null) {
                System.out.println("Account not found");
                return;
            }
            System.out.println(balance.toString());
        }

        else {
            System.out.println("Invalid option");
            return;
        }

    }
}

BankDemo class with main()

package balance;

import java.util.*;

public class BankDemo {
    public static void main(String args[]) {
        String choice, ch, operation;
        Transaction transac = new Transaction();
        Scanner sc = new Scanner(System.in);
        double amount;
        long accountNo;

        do {
            System.out.println("1. New account");
            System.out.println("2. Transaction");
            System.out.println("3. View Account Information");
            System.out.println("q. Exit");
            System.out.println("choice:");
            choice = sc.next();
            switch (choice) {

            case "1":
                double openingBalance;

                System.out.println("Enter the opening balance :");
                openingBalance = sc.nextDouble();
                transac.transaction(0, "Opening", openingBalance);
                break;

            case "2":
                System.out.println("a. Deposit");
                System.out.println("b. Withdraw");
                ch = sc.next();
                if (ch.equalsIgnoreCase("a"))
                    operation = "Deposit";
                else if (ch.equalsIgnoreCase("b"))
                    operation = "Withdraw";
                else {
                    operation = "Invalid option";
                }
                System.out.println("Account Number:");
                accountNo = sc.nextLong();
                System.out.println("Amount:");
                amount = sc.nextDouble();
                transac.transaction(accountNo, operation, amount);
                break;

            case "3":
                System.out.println("Account Number:");
                accountNo = sc.nextLong();
                operation = "showInfo";
                transac.transaction(accountNo, operation, 0);
                break;

            case "q":
                System.out.println("Thank you!");
                break;

            default:
                System.out.println("Wrong choice!!");
            }
        } while (choice != "q");
        sc.close();
    }
}

I'm looking for suggestions, improvements on readability, OOP approach, and design.

Answer 1

A couple of things I would do differently:

1. Store your list of Balance objects in a map of type HashMap<Long, Balance> using the account number as a key. This way you don't have to loop through your entire list to find the Balance you need. If you do decide to keep the loop, consider getting rid of the iterator and using the enhanced for-loop syntax.
2. Create an enum for your transaction types: enum TransactionType { OPEN, WITHDRAW, DEPOSIT INFO }
   This will protect you from typos, and in the operation() method, it will let you switch on the enum value and enable you to get rid of the long if-else control structure.
3. There's a concurrency issue. Multiple threads accessing the same Balance could result in amounts being withdrawn even if the balance is lower than zero. Probably best to synchronize on the Balance object that you're modifying.
4. Initializing a new Bank for every transaction doesn't seem right. Consider passing it in as a parameter instead.

These are the things I spotted after a cursory reading. I'll give your code a second look if I find some time later.

Answer 2

Let's get the basics right first...

After attempting to understand your implementation, these are the gotchas I can spot (some repeated from @Robby Cornelissen's observations too)

1. You can create a Balance object from another Balance object, i.e. effectively cloning and doubling accounts' numbers.
   • This smell extremely fishy at first, and it's only in the Bank class where I realize where the root cause lie: the searchAccount() method. Will explain more below.
2. Your Balance class holds a reference to a Scanner instance on System.in, something that is unused and does not belong there.
3. Your Balance class seems to know how to output to the console.
   • Generally speaking, model classes will not interact directly to outputs, be it the console or a file.
4. Your Bank class declares an ArrayList of Balance objects, there are better implementations for this as suggested by @Robby Cornelissen.
5. Every time you search for a Balance (object), you are creating a new Balance (object) just to check if the account number is the same (more below).
6. Your Bank does not keep track of Transactions.
   • In Real Life, a Bank will know about Balances and Transactions.
7. Conversely, your singular Transaction object creates a Bank for its entire lifetime and sole usage.
   • In Real Life, a Transaction will 'belong' to a Bank and it shouldn't be telling the Bank what to do.
8. Your "checks" for transactionType.equalsIgnoreCase("...") inside your operation() method are exactly what methods are for (more below).
9. Similar to a previous point, your Transaction class also seem to know how to output to the console.

---

Iterating through a collection of Balance objects

Following @Robby Cornelissen's suggestion of using a HashMap, you will not need to create 'dummy' Balance objects just to compare account numbers. Alternatively, your Balance class can have a method public boolean hasAccountNumber(long accountNum) to perform this check, instead of doing it within your Bank class.

Understanding methods

Setting a String transactionType, comparing it case-insensitive to some values inside operation(), and then performing the appropriate steps is convoluted. Just have methods like withdraw(...) or deposit(...) with the relevant arguments and call them from your BankDemo class. There is no simpler way to describe this.