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

Cash-withdrawal from an ATM

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

Problem

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,

Solution

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

Code Snippets

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();
}
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();
       ....
   }

Context

StackExchange Code Review Q#4453, answer score: 9

Revisions (0)

No revisions yet.