10
\$\begingroup\$

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<Integer> cardNum = new ArrayList<Integer>();
        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 main3 = new AccountMain();
            int selection;

            System.out.println("\nATM main menu:");
            System.out.println("1 - View account balance");
            System.out.println("2 - Withdraw funds");
            System.out.println("3 - Add funds");
            System.out.println("4 - Back to Account Menu");
            System.out.println("5 - Terminate transaction");
            System.out.print("Choice: ");
            selection = input.nextInt();

            switch(selection)
            {
            case 1:
                viewAccountInfo();
                break;
            case 2:
                withdraw();
                break;
            case 3:
                addFunds();
                break;
            case 4:
                AccountMain.selectAccount();
                break;
            case 5:
                System.out.println("Thank you for using this ATM!!! goodbye");
            }
        }

        public void viewAccountInfo()
        {
            System.out.println("Account Information:");
            System.out.println("\t--Total balance: $"+totalBal);
            System.out.println("\t--Available balance: $"+availableBal);
            drawMainMenu();
        }

        public void deposit(int depAmount)
        {
            System.out.println("\n***Please insert your money now...***");
            totalBal =totalBal +depAmount;
            availableBal =availableBal +depAmount;
        }

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

        public void addFunds()
        {
            int addSelection;

            System.out.println("Deposit funds:");
            System.out.println("1 - $20");
            System.out.println("2 - $40");
            System.out.println("3 - $60");
            System.out.println("4 - $100");
            System.out.println("5 - Back to main menu");
            System.out.print("Choice: ");
            addSelection =input.nextInt();

            switch(addSelection)
            {
            case 1:
                deposit(20);
                drawMainMenu();
                break;
            case 2:
                deposit(40);
                drawMainMenu();
                break;
            case 3:
                deposit(60);
                drawMainMenu();
                break;
            case 4:
                deposit(100);
                drawMainMenu();
                break;
            case 5:
                drawMainMenu();
                break;
            }
        }

        public void withdraw()
        {
            try{


            int withdrawSelection;

            System.out.println("Withdraw money:");
            System.out.println("1 - $20");
            System.out.println("2 - $40");
            System.out.println("3 - $60");
            System.out.println("4 - $100");
            System.out.println("5 - Back to main menu");
            System.out.print("Choice: ");
            withdrawSelection =input.nextInt();

            switch(withdrawSelection)
            {
            case 1:
                checkAmount(20);
                drawMainMenu();
                break;
            case 2:
                checkAmount(40);
                drawMainMenu();
                break;
            case 3:
                checkAmount(60);
                drawMainMenu();
                break;
            case 4:
                checkAmount(100);
                drawMainMenu();
                break;
            case 5:
                drawMainMenu();
                break;
             default:             
                    System.out.println("Invalid choice.");
                    withdraw();
            }

            }
            catch(invalidAmount err){
                System.out.println("Caught Error: " + err.getError());
                viewAccountInfo();
            }
        }

public static void query(){

    Scanner keyboard = new Scanner(System.in);


    while (!keyboard.hasNextInt()) {
        System.out.println("Invalid choice.");
        menu();
    }
    int input = keyboard.nextInt();

    if (input == 2){

        BankMainPart2 main2 = new BankMainPart2();

        System.out.println("Please enter your 5 digit card number.");
        BankMainPart2.loginCard(cardNum);
    }
    else if (input == 1){

        cardNumbers();  
    }
    else if (input == 3){
        System.out.println("Thank you, have a nice day!");
        System.exit(0);
    }
}


private static void checkNumber(int num) throws invalidNumber
//run the check activation exception
{
    Scanner keyboard = new Scanner(System.in);

    if(String.valueOf(num).length()!=5)
  {
        throw new invalidNumber("invalid number");
  }
    else  {

        cardNum.add(num);

        System.out.println("Thank you! You're card number is " +num);
        contC2();
    }
}
    private void checkAmount(int withdrawAmount) throws invalidAmount
    //run the check activation exception
    {
        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...***");
            }
    }
    public static void contC2(){
        Scanner keyboard = new Scanner(System.in);
        System.out.println("Type 'c' to return to main menu.");

        String value = keyboard.next();
        if(value.equalsIgnoreCase("c")){
        menu();
        }
        else if (!keyboard.equals('c')){
            System.out.println("Invalid Entry!");
            contC2();
        }
    }
    public static void main(String args[])
    {
        BankMain myAtm = new BankMain();
        BankMainSub sub = new BankMainSub();
        myAtm.startAtm();
    }
}

Here is my subclass...

public class BankMainSub extends BankMain {

    private double availableBal3 =500;
    private double totalBal3 =520;

    public void businessAccount()
    {
        int selection;

        System.out.println("\nATM main menu:");
        System.out.println("1 - View account balance");
        System.out.println("2 - Withdraw funds");
        System.out.println("3 - Add funds");
        System.out.println("4 - Back to Account Menu");
        System.out.println("5 - Terminate transaction");
        System.out.print("Choice: ");
        selection = input.nextInt();

        switch(selection)
        {
        case 1:
            viewAccountInfo3();
            break;
        case 2:
            withdraw3();
            break;
        case 3:
            addFunds3();
            break;
        case 4:
            AccountMain.selectAccount();
            break;
        case 5:
            System.out.println("Thank you for using this ATM!!! goodbye");
         default:             
                System.out.println("Invalid choice.");
                businessAccount();

        }
            }


    public void addFunds3()
    {
        int addSelection;

        System.out.println("Deposit funds:");
        System.out.println("1 - $20");
        System.out.println("2 - $40");
        System.out.println("3 - $60");
        System.out.println("4 - $100");
        System.out.println("5 - Back to main menu");
        System.out.print("Choice: ");
        addSelection = input.nextInt();

        switch(addSelection)
        {
        case 1:
            deposit2(20);
            businessAccount();
            break;
        case 2:
            deposit2(40);
            businessAccount();
            break;
        case 3:
            deposit2(60);
            businessAccount();
            break;
        case 4:
            deposit2(100);
            businessAccount();
            break;
        case 5:
            businessAccount();
            break;
        }
    }

    public void withdraw3()
    {
        int withdrawSelection;

        System.out.println("Withdraw money:");
        System.out.println("1 - $20");
        System.out.println("2 - $40");
        System.out.println("3 - $60");
        System.out.println("4 - $100");
        System.out.println("5 - Back to main menu");
        System.out.print("Choice: ");
        withdrawSelection =input.nextInt();

        switch(withdrawSelection)
        {
        case 1:
            checkNsf3(20);
            businessAccount();
            break;
        case 2:
            checkNsf3(40);
            businessAccount();
            break;
        case 3:
            checkNsf3(60);
            businessAccount();
            break;
        case 4:
            checkNsf3(100);
            businessAccount();
            break;
        case 5:
            businessAccount();
            break;
        }
    }

    public void viewAccountInfo3()
    {

        System.out.println("Account Information:");
        System.out.println("\t--Total balance: $"+totalBal3);
        System.out.println("\t--Available balance: $"+availableBal3);
        businessAccount();
    }

    public void deposit2(int depAmount)
    {
        System.out.println("\n***Please insert your money now...***");
        totalBal3 =totalBal3 +depAmount;
        availableBal3 =availableBal3 +depAmount;
    }

    public void checkNsf3(int withdrawAmount)
    {
        if(totalBal3 -withdrawAmount < 0)
            System.out.println("\n***ERROR!!! Insufficient funds in you accout***");
        else
        {
            totalBal3 =totalBal3 -withdrawAmount;
            availableBal3 =availableBal3 -withdrawAmount;
            System.out.println("\n***Please take your money now...***");
            }
    }
}

Here is AccountMain...

import java.util.Scanner;

public class AccountMain {


    public static void selectAccount(){


        System.out.println("Which account would you like to access?");
        System.out.println();
        System.out.println("1 = Business Account ");
        System.out.println("2 = Savings Account");
        System.out.println("3 = Checkings Account");
        System.out.println("4 = Return to Main Menu");

        menuAccount();
    }

    public static void menuAccount(){

        BankMain main = new BankMain();
        BankMainSub sub = new BankMainSub();
        BankMainPart3 main5 = new BankMainPart3();

        Scanner account = new Scanner(System.in);
        while (!account.hasNextInt()) {
            System.out.println("Invalid choice.");
            selectAccount();
        }
        int actNum = account.nextInt();


        if (actNum == 1){

            System.out.println("*Business Account*");
            sub.businessAccount();
        }

        else if (actNum == 2){

            System.out.println("*Savings Account*");
            main.drawMainMenu();
        }

        else if (actNum == 3){

            System.out.println("*Checkings Account*");
            main5.checkingsAccount();
        }

        else if (actNum == 4){
            BankMain.menu();

        }
    }
}
\$\endgroup\$
3
  • 2
    \$\begingroup\$ Rule number 1: Don't repeat yourself. Rule number 2: Read more books. Try this amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/… \$\endgroup\$ Commented Sep 27, 2012 at 20:10
  • \$\begingroup\$ To continue what @LarryBattle said: Rule #3: DON'T REPEAT YOURSELF \$\endgroup\$
    – anon
    Commented Jun 7, 2015 at 3:18
  • \$\begingroup\$ Think of your Main class as the driver for the rest of your program. Don't make that top/main class the bank. Instead Main is where you instantiate and use your bank and other objects. \$\endgroup\$
    – radarbob
    Commented Apr 11, 2017 at 20:08

2 Answers 2

10
\$\begingroup\$

A few random notes:

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

  2. 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
    }
    
  3. BankMain create new Scanners in every method although it already has one in its input field.

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

    The following is the same:

    private String error = "error";
    
  5. invalidAmount should be InvalidAmountException (Effective Java, 2nd Edition, Item 56: Adhere to generally accepted naming conventions)

  6. ArrayList<Integer> cardNum = new ArrayList<Integer>();
    

    should be

     List<Integer> cardNum = new ArrayList<Integer>();
    

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

  7. 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.

  8. 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.

\$\endgroup\$
3
  • \$\begingroup\$ Actually, the last one is not quite right. The OP had it right. In your version, everything after the if block is executed regardless of whether the Exception was thrown or not. This is NOT the desired behaviour. \$\endgroup\$ Commented Sep 28, 2012 at 9:22
  • 1
    \$\begingroup\$ @SoboLAN: When an exception is thrown the control returns to the caller or to the try/finally block so those statements do not run. \$\endgroup\$
    – palacsint
    Commented Sep 28, 2012 at 10:15
  • 1
    \$\begingroup\$ Actually yes, you're right. I would be right if that was a try/catch, not a throw new Exception statement. \$\endgroup\$ Commented Sep 28, 2012 at 11:35
0
\$\begingroup\$

You have good practice for coding in response already done.

I would point out concept concern.

If you use Java, do not use ksh or c-shell procedural paradigme.

Because of ATM constraint, all your constant values (like 20, 40, 60, 100) have to be in an Interface and never hard coded in an other place. [in a normal environment they can be put in aFile.properties]

Use an Enum for binding 1 and 20, 2 and 40 and so on

So you can print for all Enum.values(), and always with the good choice in front of the value

Read, and try samples from Effective Java, this book is inevitable, with another one to understand Pattern Design (Of all those that I was able to read, this is one who gave me the click, to understand the functioning of the design pattern) : Head first Design Pattern

It begins with Observer pattern. With it, you have to remove switch and other if revealing a way to think the solution for the future, and not transmit the problem to the objects build to solve them.

Another point : search in Effective Java StringBuilder

With those two books you will avoid all beginner mistakes, and raise rapidly Java professionnal status.

\$\endgroup\$
1
  • \$\begingroup\$ Ditto on the "Head First Design Patterns" book. In fact the "Head First" books in general are good learning tools IMO. \$\endgroup\$
    – radarbob
    Commented Apr 11, 2017 at 19:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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