1
\$\begingroup\$

I have been learning Java for the past two months, and started writing this task about simulating Bank and BankAccount to practise a bit of OOP and exception handling. Problem is I am not confident that the solution is ok. It is working fine, I just think it could be written better, but since I am a novice, I would really appreciate some advice. I cannot yet grasp the difference when I should return true/false on success/failure of a method and when to throw an exception.

Bank account Make a BankAccount class. It should have unique number and store information about its and owner - First, Last name and age.

It should store information about the balance of the account, the interest of the account and the type of the interest (complex and simple year interest).

If someone tries to create a bank account with invalid information an appropriate exception should be thrown.

It should support add, withdraw, transfer and history operations.

When the iterest is greater than 1% the withdraws are forbidden.

It should remember the last 5 operations of the account.

Make a CLI (Command Line Interpreter) for operating with the Bank accounts It should have the following functions:

create_bank_account It should prompt the user for the needed information and if everything is ok it should create a new bank account.

show_history It should prompt the user for the bank account number and show its history.

add_money It should prompt the user for the amount and the bank account number.

withdraw_money It should prompt the user for the amount and the bank account number.

transfer_money It should prompt the user for the origin and destination bank account numbers and the amount

calculate_amount It should prompt the user for bank account number and number of months. It should return the amount after the given number of months.

The Bank It should store the bank accounts.

Notes: you should create and use your own custom exceptions:

InsufficientFundsException

NonExistingBankAccountException

Here is my code: Exceptions

    package bank.exceptions;

public class NonExcistingBankAccountException extends Exception {
    public NonExcistingBankAccountException(){
        super();
    }
    public NonExcistingBankAccountException(String message){
        super(message);
    }
}

package bank.exceptions;

public class InsufficientFundsException extends Exception {
    public InsufficientFundsException(){
        super();
    }
    public InsufficientFundsException(String message){
        super(message);

    }
}

Person

package bank;

public class Person {
    private String firstName;
    private String lastName;
    private int age;

    public Person(String first, String last, int age){
        this.firstName = first;
        this.lastName = last;
        this.age = age;
    }

    public String getFirstName() {
        return firstName;
    }

    public String getLastName() {
        return lastName;
    }

    public int getAge() {
        return age;
    }
}

Enum

package bank;

enum InterestType {
    SIMPLE,
    COMPLEX;
}

Bank account

package bank;
import bank.exceptions.InsufficientFundsException;

import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Queue;
import java.util.UUID;

public class BankAccount {
    private String id;
    private Person owner;
    private double balance;
    private double interest;
    private InterestType interestType;
    private Queue<String> operations;

    public BankAccount(Person owner, double interest, InterestType interestType){
        this.owner = owner;
        this.interest = interest;
        this.interestType = interestType;
        this.id = UUID.randomUUID().toString();
        this.operations = new ArrayDeque<>(5);
    }

    public BankAccount(String firstName, String lastName, int age,double interest, InterestType interestType ){
        this(new Person(firstName,lastName,age),interest,interestType);
    }

    public void add(double money){
        balance += money;
        addOperation(String.format("%s %.2f%n", "added", money));
    }
    public boolean withdraw(double money) throws InsufficientFundsException {
        if(interest > 1){
            return false;
        }
        if(money > balance){
            throw new InsufficientFundsException("Not enough money to complete operation");
        }

        balance -= money;
        addOperation(String.format("%s %.2f%n", "withdrawn", money));
        return true;
    }

    public List<String> getHistory(){
        List<String> operationsList = new ArrayList<>(this.operations);
        addOperation(String.format("%s%n", "viewed history"));
        return operationsList;
    }

    public boolean transfer(BankAccount account, double amount) throws InsufficientFundsException {
        boolean withdrawn = withdraw(amount);
        if(withdrawn){
            account.add(amount);
            addOperation(String.format("%s %.2f to %s %n","transferred",amount,account.getId()));
        }
        return withdrawn;
    }

    public String getId() {
        return id;
    }

    public Person getOwner() {
        return owner;
    }

    public double getBalance() {
        return balance;
    }

    public double getInterest() {
        return interest;
    }

    public InterestType getInterestType() {
        return interestType;
    }

    private void addOperation(String operation){
        if(operations.size() == 5){
            operations.remove();
        }
        operations.add(operation);
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof BankAccount)) return false;
        BankAccount that = (BankAccount) o;
        return Objects.equals(getId(), that.getId());
    }

    @Override
    public int hashCode() {
        return Objects.hash(getId());
    }
}

Bank

package bank;

import bank.exceptions.InsufficientFundsException;
import bank.exceptions.NonExcistingBankAccountException;

import java.util.List;
import java.util.Set;
import java.util.HashSet;

import static java.lang.Math.pow;

public class Bank {
    private Set<BankAccount> accounts;

    public Bank(){
        accounts = new HashSet<>();
    }

    void createBankAccount(Person owner, double interest, InterestType interestType){
        BankAccount account = new BankAccount(owner,interest,interestType);
        accounts.add(account);
        System.out.println(account.getId());
    }

    void createBankAccount(String firstName, String lastName, int age, double interest, InterestType interestType){
        Person owner = new Person(firstName,lastName, age);
        createBankAccount(owner,interest,interestType);
    }

    public List<String> showOperations(String accountID) throws NonExcistingBankAccountException {
        for(BankAccount account : accounts){
            if(accountID.equals(account.getId())){
                return account.getHistory();
            }
        }
        throw new NonExcistingBankAccountException(String.format("Bank account %s does not exist.%n",accountID));
    }

    public void addMoney(String accountID, double amount) throws NonExcistingBankAccountException {
       BankAccount account = findAccount(accountID);
       account.add(amount);
    }
    public boolean withdrawMoney(String accountID,double amount) throws NonExcistingBankAccountException, InsufficientFundsException {
        BankAccount account = findAccount(accountID);
        return account.withdraw(amount);
    }

    public boolean transferMoney(String source, String destination, double amount) throws NonExcistingBankAccountException, InsufficientFundsException {
        BankAccount sourceAccount = findAccount(source);
        BankAccount destinationAccount = findAccount(destination);

        return sourceAccount.transfer(destinationAccount,amount);
    }

    public double calculateAmount(String accountID, int months) throws NonExcistingBankAccountException{
        BankAccount account = findAccount(accountID);
        double years = ((double) months)/12;
        switch(account.getInterestType()) {
            case SIMPLE:
                return account.getBalance() * account.getInterest() * years;
            case COMPLEX:
                return account.getBalance() * (pow((1 + account.getInterest()), years) - 1);
            default:
                throw new IllegalArgumentException();
        }
    }
    private BankAccount findAccount(String accountID) throws NonExcistingBankAccountException{
        for(BankAccount account : accounts){
            if(accountID.equals(account.getId())){
                return account;
            }
        }
        throw new NonExcistingBankAccountException(String.format("Bank account %s does not exist.%n",accountID));
    }
}

The command line interpreter

package bank;

import bank.exceptions.InsufficientFundsException;
import bank.exceptions.NonExcistingBankAccountException;

import java.io.IOException;
import java.util.InputMismatchException;

import java.util.List;
import java.util.Scanner;

public class CommandLineInterpreter{

     private Bank bank;

     private static final String SUCCESS = "Operation completed.";
     private static final String PROMPT_BANK_ACCOUNT = "Enter bank account ID: ";
     private static final String FAIL = "Operation failed.";
     private static final String CANNOT_WITHDRAW = "Either you do not have enough money to complete the operation or your interest is greater than 1% in which case withdraws are forbidden.";
     private static final String PROMPT_AMOUNT = "Enter amount: ";

     public CommandLineInterpreter(){
         bank = new Bank();
     }

    private void showOpitons(){
        System.out.println("Choose an option: ");
        System.out.println("1: Create a bank account.");
        System.out.println("2: Show history.");
        System.out.println("3: Deposit money.");
        System.out.println("4: Withdraw money");
        System.out.println("5: Transfer money.");
        System.out.println("6: Calculate amount");
        System.out.println("7: Exit");
    }

    public void start() throws IOException {
        int option;
        Scanner reader = new Scanner(System.in);
        do{
            showOpitons();
           try{
            option = reader.nextInt();
            reader.nextLine();

            if(option<1 || option > 6) {
                break;
            }

                switch (option) {
                    case 1:
                        createBankAccount(reader);
                        break;
                    case 2:
                        showHistory(reader);
                        break;
                    case 3:
                        addMoney(reader);
                        break;
                    case 4:
                        withdrawMoney(reader);
                        break;
                    case 5:
                        transferMoney(reader);
                        break;
                    case 6:
                        calculateAmount(reader);
                        break;
                }
            }catch(InputMismatchException e){
               System.err.println("Invalid argument. Try again.");
               reader.next();
           }catch(Exception e){
                System.err.println(e.getMessage());
            }
        } while(true);
        reader.close();
    }
    private Person readOwner(Scanner reader){
        System.out.println("Enter owner's name: ");
        String name = reader.nextLine();
        String names[] = name.split("\\s");

        System.out.println("Enter owner's age: ");
        int age = reader.nextInt();
        return new Person(names[0],names[1],age);
    }

    private InterestType readInterestType(Scanner reader){
        System.out.println("Choose interest type:\n1. Simple\n2. Complex");
        short option = reader.nextShort();

        switch(option){
            case 1:
                return InterestType.SIMPLE;
            case 2:
                return InterestType.COMPLEX;
            default:
                throw new IllegalArgumentException();
        }
    }

    public void createBankAccount(Scanner reader) {
        Person owner = readOwner(reader);

        System.out.println("Enter interest rate: ");
        double interest = reader.nextDouble();

        InterestType interestType = readInterestType(reader);
        bank.createBankAccount(owner,interest,interestType);
    }

    public void showHistory(Scanner reader) throws NonExcistingBankAccountException{
        System.out.println(PROMPT_BANK_ACCOUNT);
        String account = reader.nextLine();

        List<String> operations = bank.showOperations(account);
        for(String operation : operations){
            System.out.println(operation);
        }
     }

    public void addMoney(Scanner reader) throws NonExcistingBankAccountException{
        System.out.println(PROMPT_BANK_ACCOUNT);
        String account = reader.nextLine();

        System.out.println(promptAmount);
        double amount = reader.nextDouble();
        bank.addMoney(account, amount);
        System.out.println(success);
    }

    public void withdrawMoney(Scanner reader) throws InsufficientFundsException, NonExcistingBankAccountException {
        System.out.println(PROMPT_BANK_ACCOUNT);
        String account = reader.nextLine();

        System.out.println(PROMPT_AMOUNT);
        double amount = reader.nextDouble();

        if(bank.withdrawMoney(account, amount)){
            System.out.println(SUCCESS);
        }else {
            System.err.println(FAIL);
            System.err.println(CANNOT_WITHDRAW);
        }
    }

    public void transferMoney(Scanner reader) throws InsufficientFundsException, NonExcistingBankAccountException{
        System.out.println("Enter source bank account ID: ");
        String source = reader.nextLine();

        System.out.println("Enter destination bank account ID: ");
        String destination = reader.nextLine();

        System.out.println(PROMPT_AMOUNT);
        double amount = reader.nextDouble();

        if(bank.transferMoney(source,destination , amount)){
            System.out.println(SUCCESS);
        }else {
            System.err.println(FAIL);
            System.err.println(CANNOT_WITHDRAW);
        }
     }
    public void calculateAmount(Scanner reader) throws NonExcistingBankAccountException {
        System.out.println(PROMPT_BANK_ACCOUNT);
        String account = reader.nextLine();

        System.out.println("Enter months for which you want to calculate the amount: ");
        int months = reader.nextInt();

        System.out.println(String.format("%.2f",bank.calculateAmount(account, months)));
    }
}
\$\endgroup\$
  • \$\begingroup\$ Nice code. Parallel usage of the same bank account (concurrency) is probably a future subject. double is just an approximation of a sum of (negative) powers of 2 and hence might be problematic. BigDecimal can set the "scale", number of decimals. private static final String SUCCESS = ... \$\endgroup\$ – Joop Eggen Dec 19 '18 at 11:10
  • \$\begingroup\$ Thank you. Yes, I will keep upgrading it, as long as I have laid the foundations right. \$\endgroup\$ – Michaela Dec 19 '18 at 11:23
0
\$\begingroup\$

This is already a good piece of code. Good job.

I can imagine some improvements for your code:

Exceptions:

Except a typo into the name of NonExistingBankAccountException I would suggest to move the construction of the message in the exception itself. So that you will always have the same message and your code is more concise:

new NonExistingBankAccountException(String accountId)

Bank:

There is a good Object Oriented case study into your calculateAmount method: How can you get rid of this switch ?

The switch expression is useful but may be a code smell in some case. And it can be a code smell in your case.

Imagine that you want to add another InterestType. You should change this enum AND your Bank class. What can you do to preserve the single responsibility principe ?

BankAccount:

For the withdraw forbidden rule when interest is greater than 1% I will also use an exception but provide a test method to verify if we can withdraw. Having a interest under % is then a pre-condition of the withdraw method.

For the operations, instead of storing a list of strings, you can view the operations as executable treatments on the account. So you do not have to maintain the state of the account but storing his operations history and executing them to have the current state. This a kind of event sourcing that I suggest in another similar question : https://codereview.stackexchange.com/a/188370/115154

CommandLineInterpreter:

There is a typo in showOpitons.

Person:

It is usually easier to store the birth date instead of the age. Because the age change in time while the birth date don't. And you can always compute the age from the birth date.

Your question:

I cannot yet grasp the difference when I should return true/false on success/failure of a method and when to throw an exception.

I would just say that exceptions are for exceptional cases, an exception occur when you encounter and invalid state but they should never be used to control the flow of your program while booleans can.

This is sometimes more clear when you use reason with pre and post conditions. An exception is thrown when a pre condition is not matched:

/**
 * Attempt to remove the given amount from this account balance.
 * @param amount a positive double indicating the amount to be withdrawn. `> -1`.
 * @return true when the amount has been withdrawn. Otherwise false.
 * @throw InvalidAmountException iff the amount is a nagtive number.
boolean withdraw(double amount) {
  // ...
}

The param has a precondition of being positive. If this precondition is not met, then you may receive an exception. However for some reasons it happens that everything is fine but the withdrawal cannot be executed. Then you get a false because this is no due to a violation of the contract or invalid state.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the detailed answered! As for the InterestType class I hope you are talking about doing something like this: enum InterestType { SIMPLE { public double calculateAmount ( double amount, double interest, double years ) { //.. In which case the switch will disappear? I just was not aware of being able to use enums in that way. \$\endgroup\$ – Michaela Dec 19 '18 at 15:30
  • \$\begingroup\$ Yes, that was my idea. But there may be other solutions like having one BankAccount subclass for each type of intereset. \$\endgroup\$ – gervais.b Dec 20 '18 at 6:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.