added atm LLD feeback for improvement

Author: codeimmortal

atm-machine/atm/Account.go

@@ -24,20 +24,19 @@ func AccountFactory(accType AccountType) (Account, error) {
 	}
 }
 
-type SavingAccount struct {
+type BaseAccount struct {
 	acctNo  string
 	balance float64
 }
 
-func (s *SavingAccount) GetBalance() float64 {
-	return s.balance
+func (b *BaseAccount) GetBalance() float64 {
+	return b.balance
 }
 
-type CurrentAccount struct {
-	acctNo  string
-	balance float64
+type SavingAccount struct {
+	BaseAccount
 }
 
-func (c *CurrentAccount) GetBalance() float64 {
-	return c.balance
+type CurrentAccount struct {
+	BaseAccount
 }

atm-machine/atm/atm.go

@@ -7,13 +7,13 @@ import (
 )
 
 type WithdrawNote struct {
-	FiveHundread int
-	Hundread     int
-	Left         int
+	FiveHundred int
+	Hundred     int
+	Left        int
 }
 
 type ATM struct {
-	countOFNotes    map[string]int
+	countOfNotes    map[string]int
 	card            model.Card
 	account         Account
 	uiOption        []string
@@ -30,7 +30,7 @@ func (a *ATM) SetState(s ATMState) {
 	a.currentState = s
 }
 
-func (a *ATM) ResetAtm() {
+func (a *ATM) CleanPreviousAtmTransaction() {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	a.WithdrawAs = &WithdrawNote{}
@@ -41,7 +41,7 @@ func (a *ATM) ResetAtm() {
 func (a *ATM) PrintMoney() {
 	a.mu.Lock()
 	defer a.mu.Unlock()
-	fmt.Printf("\n500 note present:%d, 100 not present:%d", a.countOFNotes["500"], a.countOFNotes["100"])
+	fmt.Printf("\n500 note present:%d, 100 not present:%d", a.countOfNotes["500"], a.countOfNotes["100"])
 }
 
 func (a *ATM) StateName() string {
@@ -64,8 +64,15 @@ func (a *ATM) GetCardDetail() error {
 
 func (a *ATM) DispenserAmount() error {
 	a.mu.Lock()
-	defer a.mu.Unlock()
-	return a.currentState.DispenserAmount()
+	if err := a.currentState.DispenserAmount(); err != nil {
+		return err
+	}
+
+	a.mu.Unlock()
+
+	a.CleanPreviousAtmTransaction()
+
+	return nil
 }
 
 func (a *ATM) SelectAccount() error {
@@ -77,7 +84,7 @@ func (a *ATM) SelectAccount() error {
 func (a *ATM) Execute(operation func() error) {
 	err := operation()
 	if err != nil {
-		a.ResetAtm()
+		a.CleanPreviousAtmTransaction()
 		a.SetState(a.insertCard)
 		fmt.Println("All operation will be nil operation:")
 		fmt.Println("Error while operation:", err.Error())
@@ -86,7 +93,7 @@ func (a *ATM) Execute(operation func() error) {
 
 func NewATM() *ATM {
 	atm := &ATM{
-		countOFNotes: map[string]int{
+		countOfNotes: map[string]int{
 			"500": 1000,
 			"200": 2000,
 			"100": 1000,
@@ -106,10 +113,6 @@ func NewATM() *ATM {
 		atm: atm,
 	}
 
-	atm.readCard = &ReadCard{
-		atm: atm,
-	}
-
 	atm.dispenserAmount = &DispenserAmount{
 		atm: atm,
 	}

atm-machine/atm/card_reader.go

@@ -2,6 +2,10 @@ package atm
 
 import "atm-machine/model"
 
+const (
+	InvalidCard = "invalid"
+)
+
 // This will be actuall implementation of card reader // i am returning a card from here
 type CardReader struct {
 }
@@ -13,7 +17,7 @@ func NewCardReader() CardReader {
 // for now it will return some dummy card depend of input for acting as multiple car
 func (c CardReader) ReadCard(cardType string) model.Card {
 	switch cardType {
-	case "invalid":
+	case InvalidCard:
 		return model.Card{
 			Status: model.NoCard,
 		}

atm-machine/atm/insert_pin.go

@@ -5,12 +5,12 @@ import (
 	"fmt"
 )
 
-type InsetPin struct {
+type InsertPin struct {
 	atm *ATM
 	ATMAbstract
 }
 
-func (s *InsetPin) InsertPin() error {
+func (s *InsertPin) InsertPin() error {
 	// logic to valid pin, we can call api client inside it, i will return sucess and error depend on input
 	fmt.Println("Enter 4 letter pin for card:\n")
 
@@ -25,6 +25,6 @@ func (s *InsetPin) InsertPin() error {
 	}
 }
 
-func (d *InsetPin) StateName() string {
+func (d *InsertPin) StateName() string {
 	return "InsetPin"
 }

atm-machine/atm/select_account.go

@@ -17,11 +17,13 @@ func (s *SelectAccount) SelectAccount() error {
 	fmt.Println("Select account type SAVING OR CURRENT:\n")
 	fmt.Scanf("%s", &accountType)
 
-	s.atm.account, err = AccountFactory(AccountType(accountType))
+	if s.atm.account, err = AccountFactory(AccountType(accountType)); err != nil {
+		return err
+	}
 
 	s.atm.SetState(s.atm.dispenserAmount)
 
-	return err
+	return nil
 
 }
 

atm-machine/atm/withdraw.go

@@ -5,30 +5,30 @@ type Withdraw interface {
 }
 
 func NewWithDrawPipeline() Withdraw {
-	return fiveHundreadWithdraw{
-		next: hundreadHundreadWithdraw{},
+	return fiveHundredWithdraw{
+		next: oneHundredWithdraw{},
 	}
 }
 
-type fiveHundreadWithdraw struct {
+type fiveHundredWithdraw struct {
 	next Withdraw
 }
 
-func (f fiveHundreadWithdraw) ProcessAmount(atm *ATM, atmamount float64) {
+func (f fiveHundredWithdraw) ProcessAmount(atm *ATM, atmamount float64) {
 	div := atmamount / 500
 	rem := int(atmamount) % 500
-	atm.WithdrawAs.FiveHundread = int(div)
+	atm.WithdrawAs.FiveHundred = int(div)
 	f.next.ProcessAmount(atm, float64(rem))
 }
 
-type hundreadHundreadWithdraw struct {
+type oneHundredWithdraw struct {
 	next Withdraw
 }
 
-func (h hundreadHundreadWithdraw) ProcessAmount(atm *ATM, atmamount float64) {
+func (h oneHundredWithdraw) ProcessAmount(atm *ATM, atmamount float64) {
 	div := atmamount / 100
 	rem := int(atmamount) % 100
-	atm.WithdrawAs.Hundread = int(div)
+	atm.WithdrawAs.Hundred = int(div)
 	atm.WithdrawAs.Left = rem
 
 }

atm-machine/atm/withdrawAmount.go

@@ -23,20 +23,19 @@ func (w *DispenserAmount) DispenserAmount() error {
 	if w.atm.WithdrawAs.Left > 0 {
 		return errors.New("This amount is not divisible of 100")
 	} else {
-		if w.atm.countOFNotes["500"] >= w.atm.WithdrawAs.FiveHundread {
-			w.atm.countOFNotes["500"] = w.atm.countOFNotes["500"] - w.atm.WithdrawAs.FiveHundread
+		if w.atm.countOfNotes["500"] >= w.atm.WithdrawAs.FiveHundred {
+			w.atm.countOfNotes["500"] = w.atm.countOfNotes["500"] - w.atm.WithdrawAs.FiveHundred
 		} else {
 			return errors.New("Bank don't have enough fund 500")
 		}
 
-		if w.atm.countOFNotes["100"] >= w.atm.WithdrawAs.Hundread {
-			w.atm.countOFNotes["100"] = w.atm.countOFNotes["100"] - w.atm.WithdrawAs.Hundread
+		if w.atm.countOfNotes["100"] >= w.atm.WithdrawAs.Hundred {
+			w.atm.countOfNotes["100"] = w.atm.countOfNotes["100"] - w.atm.WithdrawAs.Hundred
 		} else {
 			return errors.New("Bank don't have enough fund 100")
 		}
 	}
 
-	w.atm.ResetAtm()
 	w.atm.SetState(w.atm.insertCard)
 
 	return nil

atm-machine/main.go

@@ -13,7 +13,7 @@ func main() {
 		var runProcess string
 
 		fmt.Printf("\nATM state:%s\n", atm.StateName())
-		fmt.Println("Input do you want to proceed, press n to exist")
+		fmt.Println("Input do you want to proceed, press n to exit")
 		fmt.Scanf("%s", &runProcess)
 
 		fmt.Println(runProcess)