Skip to main content
added 291 characters in body
Source Link

Specifically, create a Money class which can add, subtract other Money objects. Internally to Money, you should use a BigDecimal to represent the amounts. But certainly not a double or a float because of their lack of precision. Try adding 0.1 and 0.2. You won't get 0.3: you'll get 0.30000000000000004. BigDecimal avoids falling into that trap, and if you use add 0.1 and 0.2, you'll really get 0.3 and not any other number.

Specifically, create a Money class which can add, subtract other Money objects. Internally to Money, you should use a BigDecimal to represent the amounts.

Specifically, create a Money class which can add, subtract other Money objects. Internally to Money, you should use a BigDecimal to represent the amounts. But certainly not a double or a float because of their lack of precision. Try adding 0.1 and 0.2. You won't get 0.3: you'll get 0.30000000000000004. BigDecimal avoids falling into that trap, and if you use add 0.1 and 0.2, you'll really get 0.3 and not any other number.

Source Link

You have bugs

  • A malicious user can enter negative numbers to withdraw negative numbers, this would result in adding money to their account.
  • A malicious user can also enter negative numbers to "steal" money from another account when transferring money with such negative number.

Create specific types for your business objects

Specifically, create a Money class which can add, subtract other Money objects. Internally to Money, you should use a BigDecimal to represent the amounts.

Account should contain a balance represented by a Money, but that balance should never go in the negative.

Create a PhoneNumber class. This way, you can lookup by phone number with an appropriate object. The validation will happen only when you create the object, and internally you can represent a PhoneNumber as you wish, but I recommend a String, as you used. Here, since you check if a phone number exists, don't forget to reimplement equals and hashcode. Your IDE should help you make those methods correctly.

Use exceptions in your code

You asked to use exception, so just use them.

What are the possible issues in your code?

  1. That an account doesn't exist.
  2. That an amount is negative to avoid bugs.
  3. That an amount is bigger than the balance when trying to withdraw.

So let's just write those exception:

UnknownAccountException.java

public class UnknownAccountException extends RuntimeException {
  public UnknownAccountException(String message) {
    super(message);
  }
}

NegativeAmountException.java

public class NegativeAmountException extends RuntimeException {
  public NegativeAmountException(String message) {
    super(message);
  }
}

InsufficientBalanceException.java

public class InsufficientBalanceException extends RuntimeException {
  public InsufficientBalanceException(String message) {
    super(message);
  }
}

Your actions should be simple

Here your actions are complex. They do more than one thing. If you deposit money, not only do you add money there, but you also print the balance afterwards. A method named depositMoney should only deposit money, so avoid printing the balance. If at all, print the balance in the UI, that is in your case, your main class.

Also, you should keep your actions consistent. If you create a depositMoney, why don't you use it when you transfer money?

Your account's money-related methods should be as simple as this:

private void checkPositiveAmount(Money amount) {
  if (amount.asBigDecimal().compareTo(BigDecimal.ZERO) < 0) {
    throw new NegativeAmountException("Negative amount: " + amount);
  }
}

private void checkSufficientFunds(Money amount) {
  if (balance.compareTo(amount) < 0) {
    throw new InsufficientBalanceException("Not enough funds to withdraw: " + amount);
  }
}

public void depositMoney(Money amount) {
  checkPositiveAmount(amount);
  balance = balance.add(amount);
}

public void withdrawMoney(Money amount) {
  checkPositiveAmount(amount);
  checkSufficientFunds(amount);
  balance = balance.subtract(amount);
}

public void transferMoneyTo(Money amount, Account destination) {
  withdrawMoney(amount);
  destination.depositMoney(amount);
}

With such simple actions, the risk of errors is way lower than previously.

Your UI is too complex

Your UI contains few methods, and those are so big that you don't know how to name those methods. Also, you make several errors in your main interface.

Reduce your UI in small methods. Small methods means less chance to mess up.

Here's how I'd decompose your Main class:

public class Main implements Runnable {
  public static void main(String[] args) {
    new Main().run();
  }

  private final Scanner scanner = new Scanner(System.in);
  private final Bank bank = new Bank();

  @Override
  public void run() {
    do {
      int option = selectAccountOption();
      switch (option) {
      case 1:
        Account account = register();
        manageAccount(account);
        break;
      case 2:
        Account account = login();
        manageAccount(account);
        break;
      case 3:
        exit();
        return;
      }
    } while (true);
  }
  
  private int selectAccountOption() {
    do {
      System.out.println("Hello, press: ");
      System.out.println("1. Register a new account");
      System.out.println("2. Log in to your account");
      System.out.println("3. Exit");
      int option = scanner.nextInt();
      if (1 <= option && option <= 3) {
        return option;
      }
      System.out.println("Your input was not recognized. Please try again.");
    } while (true);  
  }

  private Account register() {
    String firstName = readFirstName();
    String lastName = readLastName();
    PhoneNumber phoneNumber = readPhoneNumber();
    Account account = new Account(firstName, lastName, phoneNumber);
    printAccountId(account);
    return account;
  }

  private Account login() {
    PhoneNumber phoneNumber = readPhoneNumber();
    Account account = lookupAccount(phoneNumber);
    return account;
  }

  private void manageAccount(Account account) {
    do {
      printBalance(account);
      int option = selectManageAccountOption();
      switch (option) {
      case 1:
        info(account);
        break;
      case 2:
        deposit(account);
        break;
      case 3:
        withdraw(account);
        break;
      case 4:
        transfer(account);
        break;
      case 5:
        return;
      }
    } while (true);
  }

  private Account lookupAccount(PhoneNumber phoneNumber) {
    ...
  }

  private Account lookupAccount(PhoneNumber phoneNumber, int id) {
    ...
  }
  
  private void info(Account account) {
    ...
  }

  private void deposit(Account account) {
    Money amount = readAmountToDeposit();
    try {
      account.depositMoney(amount);
    } catch(NegativeAmountException e) {
      System.out.println("Illegal amount. The amount was not deposited.");
    }
  }

  private void withdraw(Account account) {
    Money amount = readAmountToWithdraw();
    try {
      account.withdrawMoney(amount);
    } catch (NegativeAmountException e) {
      System.out.println("Illegal amount. The amount was not withdrawn");
    } catch (InsufficientBalanceException e) {
      System.out.println("Amount too high. The amount was not withdrawn.");
    }
  }
  
  private void transfer() {
    PhoneNumber phoneNumber = readPhoneNumber();
    Account destination = lookupAccount(phoneNumber);
    Amount amount = readAmountToTransfer();
    try {
      account.transferMoney(amount, destination);
    } catch (NegativeAmountException e) {
      System.out.println("Illegal amount. The amount was not transferred");
    } catch (InsufficientBalanceException e) {
      System.out.println("Amount too high. The amount was not transferred.");
    }
  }

}

There are still plenty of methods, but I tried to have one type of each method used, so you should be able to replicate those to fill the missing methods.