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

Ruby banking system program

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

Problem

As a beginner I'd like to get any kind of feedback. The more the better. Any optimization and style mistakes?

class Account
      attr_reader :name, :balance

      def initialize(name, balance=100)
        @name = name
        @balance = balance
      end   

      private

      def pin
      @pin = 789 
      end  

      def pin_check
        puts "Welcome to the banking system, #{@name}!\n" + "To access your account, input PIN please"
        @input_pin = gets.chomp.to_i
      end      

     def pin_error 
        return "Access denied: incorrect PIN."
     end

    public

      def access
        if pin_check == pin
          puts "Input d: to deposit money, s: to show balance or w: to withdraw money"
          action = gets.chomp.downcase
            case action
            when "d"
            deposit
            when "s"
            display_balance
            when "w"
           withdraw
            else
        puts "Try again"
          end
        else puts pin_error
        end    
      end

      def amount
        puts "Input the amount"
        @money = gets.to_i
      end

      def over_error
        print "You don't own that kind of money, dude! Your balance: $#{@balance}"
      end

      def deposit
       @balance += amount
        puts "Deposited: $#{@money}. Updated balance: $#{@balance}."
      end

      def display_balance
        puts "Balance: $#{@balance}."   
      end

      def withdraw
        if  amount <= @balance
          @balance -= @money
           puts "Withdrew: $#{@money}. Updated balance: $#{@balance}."  
        else
            puts over_error
        end
      end
    end  

    checking_account = Account.new("James Bond", 520_000)
    checking_account.access

Solution

There are a few things that I would do differently and/or change:

-
I usually find it easier to put all the public methods at the top and the private methods at the bottom instead of mixing them. This is also what I see most people do.

-
Be consistent. In pin_error you return an error message, in over_error you print the message (elsewhere you use puts). I would make both methods just puts the error message.

-
I would not assign an instance variable in amount just return the value and then change withdraw to use a local variable. I would also call it get_amount or something since it does more than just return an amount.

def withdraw
  amt = get_amount
  if  amt <= @balance
    @balance -= amt
    puts "Withdrew: $#{amt}. Updated balance: $#{@balance}."  
  else
    over_error
  end
end


-
The pin method is odd in that it returns the bin but also sets an instance variable unnecessarily. I would pass the pin into the constructor.

-
I would make pin_check actually check if the pin is correct and return true or false. I would also give it an active name, with a bang, like check_pin!

Code Snippets

def withdraw
  amt = get_amount
  if  amt <= @balance
    @balance -= amt
    puts "Withdrew: $#{amt}. Updated balance: $#{@balance}."  
  else
    over_error
  end
end

Context

StackExchange Code Review Q#162405, answer score: 5

Revisions (0)

No revisions yet.