Cash-withdrawal from an ATM

Question

I'm seeking review comments (design, performance, etc.) based on the problem statement:

Write a CashWithDrawal function from an ATM which based on user specified amount dispenses bank notes. Ensure that the following is taken care of

• Minimum number of bank notes are dispensed
• Availability of various denominations in the ATM is maintained
• Code should be flexible to take care of any bank denominations as long as it is a multiple of 10
• Code should support parallel withdrawals i.e. two or more customers can withdraw money simultaneously
• Take care of exceptional situation

---

package com.assignment.atm;

import java.util.Scanner;


/**
 * The Class Main.
 */
public class Main
{

    /**
     * The main method.
     * 
     * @param args the arguments
     */
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        System.out.println("Enter Amount to be withdrawn: ");
        int amount = input.nextInt();
        if(amount%10!=0){
            System.out.println("Please enter the amount in multiples of 10");
        }
        else{
            ATM atm = new  ATM(amount);
            ATM.calcTotalCorpus();
            Thread t1 = new Thread(atm);
            t1.start();
            /*ATM.calcTotalCorpus();
            Thread t1 = new Thread(new ATM(1200));
            Thread t2 = new Thread(new ATM(2400));
            t1.start();
            t2.start();
            try{
            t2.sleep(2000);
            }
            catch(Exception e){
                //Sysout Exception trace
            }*/
        }




    }
}

---

package com.assignment.atm;


/**
 * The Class ATM.
 */
class ATM implements Runnable{

    /** The Constant Currency Denominations. */
    protected static final int[] currDenom = { 1000, 500, 100, 50 , 10 };

    /** The Number of Currencies of each type*/
    protected static int[] currNo = {1,4,2,2,10};

    /** The count. */
    protected  int[] count = { 0, 0, 0, 0 ,0};

    /** The total corpus. */
    protected static int totalCorpus = 0;

    /** The amount. */
    protected  int amount=0;

    /**
     * Instantiates a new aTM.
     * 
     * @param amount The Amount of type int
     */
    public ATM(int amount){
        this.amount=amount;
    }


    /**
     * Calc total corpus.
     */
    public static void calcTotalCorpus(){       
        for(int i = 0; i < currDenom.length; i++){
            totalCorpus=totalCorpus+currDenom[i]*currNo[i];
        }
    }

    /**
     * Withdraw cash.
     */
    public  synchronized  void  withdrawCash(){
        if(amount<=totalCorpus){
            for (int i = 0; i < currDenom.length; i++) {
                if (currDenom[i] <= amount) {//If the amount is less than the currDenom[i] then that particular denomination cannot be dispensed
                    int noteCount = amount / currDenom[i];
                    if(currNo[i]>0){//To check whether the ATM Vault is left with the currency denomination under iteration
                        //If the Note Count is greater than the number of notes in ATM vault for that particular denomination then utilize all of them 
                        count[i] = noteCount>=currNo[i]?currNo[i]:noteCount;
                        currNo[i] =  noteCount>=currNo[i]?0:currNo[i]- noteCount;
                        //Deduct the total corpus left in the ATM Vault with the cash being dispensed in this iteration
                        totalCorpus=totalCorpus-(count[i]*currDenom[i]);
                        //Calculate the amount that need to be addressed in the next iterations
                        amount = amount -(count[i]*currDenom[i]);
                    }                
                }
            }
            displayNotes();
            displayLeftNotes();

        }
        else{
            System.out.println("Unable to dispense cash at this moment for this big amount");
        }

    }

    /**
     * 
     * 
     */
    public void run()
    {
        withdrawCash();

    }

    /**
     * Display notes.
     */
    private void displayNotes(){
        for (int i = 0; i < count.length; i++) {
            if (count[i] != 0) {
                System.out.println(currDenom[i] + " * " + count[i] + " = "+ (currDenom[i] * count[i]));
            }
        }
    }

    /**
     * Display left notes.
     */
    private void displayLeftNotes(){
        for(int i = 0; i < currDenom.length; i++){
            System.out.println("Notes of "+currDenom[i]+" left are "+currNo[i]);
        }

    }
}

Answer 1

(1) API is a bit odd. Why do I need to call some static methods after I instatiated an object and want to use it already? ATM atm = new ATM(amount); ATM.calcTotalCorpus();

(2) There is a slight mess with access modifiers. For example, Why protected ? protected int amount=0; And why is this method public ? public synchronized void withdrawCash()

(3)This method void withdrawCash() is full of coments but it would be a lot better to divide this method into a smaller functions to make it more readable. Consider

public  synchronized  void  withdrawCash(){
if(isAmountSufficient()){
    for (int i = 0; i < currDenom.length; i++) {
        if (canDispenseDenomination()) {
            getNoteCount();
            if(canProceedWithIterations()){
                doCalculations();
                deductTotalCorpuse()
                calculateAmmountForNextIteration();
            }                
        }
    }
    displayNotes();
    displayLeftNotes();

}
else{
   reportInsufficientAmmount();
}

(4) There is also some odd synchronization. The presense of 2 customers means that you have 2 threads which work with a single ATM however in your case you have only one thread for a single ATM and no concurrent access to this ATM from different customers. I would do it in the follwoing way:

 public class Consumer extends Thread {
          private final ATM atm;    
          public Consumer (ATM atm) {this.atm = atm;} 

          public void Run() {
             this.atm.dispense(10);
             Thread.sleep(10);
             this.atm.dispense(20);
          }
   } 

   public static void main(String[] args) {
       ATM atm = new ATM(1000);
       Consumer c1 = new Consumer(atm);
       Consumer c2 = new Consumer(atm);
       c1.Start();
       c2.Start();
       ....
   }

The dispsense() function can implement the logic from withdrawCash() function.

Answer 2

In this type of problem (money handling), I would zealously focus on exceptional cases first and then ensure that the code is well structured and easy to understand.

First thing that hit me was that you have no check for negative number input.

If you go with the ATM class with amount in constructor, I would suggest to add a input checking in the constructor - for starters testing that input is positive and is multiple of ten. If input is invalid then IllegalArgumentException (or your own subclass of it) flies.

Furthermore, I would make the amount field private final - and then change the rest of the code (use a method variable to keep track of the amount left to dispose) and eventually design to accommodate this change. The reasoning being that I prefer to be extra paranoid with a sensitive data like this.