HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Bank ATM program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bankatmprogram

Problem

I'm new to Java and have been reading Java for Dummies and other ones as well. I've started building this program like a week ago. I'm sure it's very messy. Just seeing if someone can clean it up and show easier ways of doing things. I have like 4 classes, but I'll just post the main class and a subclass for now.

Here is my BankMain class...

```
import java.util.ArrayList;
import java.util.Scanner;

public class BankMain
{
private double availableBal =80;
private double totalBal =100;

static ArrayList cardNum = new ArrayList();
static Scanner input = new Scanner(System.in);

private String error; //String the error from the exception
{
error = "error";
}
public static void cardNumbers(){
Scanner input = new Scanner(System.in);
try{

System.out.println("Please select a 5 digit card number");
int num = input.nextInt();
checkNumber(num);
}
catch(invalidNumber err){

System.out.println("Caught Error: " + err.getError());
contC();
}
}
public static void contC(){
Scanner keyboard = new Scanner(System.in);
System.out.println("Type 'c' to enter number again.");

String value = keyboard.next();
if(value.equalsIgnoreCase("c")){
cardNumbers();

}

else if (!keyboard.equals('c')){

System.out.println("Invalid Entry!");
}
}
public static void menu(){

System.out.println("ATM Menu:");
System.out.println();
System.out.println("1 = Create Account");
System.out.println("2 = Account Login");
System.out.println("3 = Exit ATM");
query();
}

public void startAtm()
{
menu();
}
public void drawMainMenu()
{
AccountMain ma

Solution

A few random notes:

-
Floating point values are not precise. You should use BigDecimals for the balance instead of double. Some useful reading:

  • Why not use Double or Float to represent currency?



  • Effective Java, 2nd Edition, Item 48: Avoid float and double if exact answers are required



-
Lots of methods calls each other recursively. Some possible code paths:

  • menu -> query -> menu



  • query -> cardNumbers -> checkNumber -> contC2 -> menu



If a user uses the application long enough they will get a StackOverflowError sooner or later. You should use loops to get the user's input and don't call again recursively the menu printer method from the input handler.

A possible main menu method:

while (true) {
    print main menu
    read input
    if input invalid {
        continue
    }
    handle input (call submenu methods)
}


A possible submenu method:

while (true) {
    print submenu
    read input
    if input invalid {
        continue
    }
    if user chose exit submenu {
        return
    }
    handle input
}


-
BankMain create new Scanners in every method although it already has one in its input field.

-
private String error; // String the error from the exception
{
    error = "error";
}


The following is the same:

private String error = "error";


-
invalidAmount should be InvalidAmountException (Effective Java, 2nd Edition, Item 56: Adhere to generally accepted naming conventions)

-
ArrayList cardNum = new ArrayList();


should be

List cardNum = new ArrayList();


(Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces)

-
Comments like this are really hard to read on smaller screens because of the horizontal scrolling and the unnecessary spaces:

private static void checkNumber(int num) throws invalidNumber               //run the check activation exception


You could put it above the method declaration.

-

if (totalBal - withdrawAmount < 0) {
    throw new invalidAmount("\n***ERROR!!! Insufficient funds in you accout***");
} else {
    totalBal = totalBal - withdrawAmount;
    availableBal = availableBal - withdrawAmount;
    System.out.println("\n***Please take your money now...***");
}


If you throw an exception the else block is unnecessary, it could be this:

if (totalBal - withdrawAmount < 0) {
    throw new invalidAmount("\n***ERROR!!! Insufficient funds in you accout***");
}
totalBal = totalBal - withdrawAmount;
availableBal = availableBal - withdrawAmount;
System.out.println("\n***Please take your money now...***");


It's often called as guard clause.

Code Snippets

while (true) {
    print main menu
    read input
    if input invalid {
        continue
    }
    handle input (call submenu methods)
}
while (true) {
    print submenu
    read input
    if input invalid {
        continue
    }
    if user chose exit submenu {
        return
    }
    handle input
}
private String error; // String the error from the exception
{
    error = "error";
}
private String error = "error";
ArrayList<Integer> cardNum = new ArrayList<Integer>();

Context

StackExchange Code Review Q#15994, answer score: 10

Revisions (0)

No revisions yet.