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?
- That an account doesn't exist.
- That an amount is negative to avoid bugs.
- 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.