patternrubyMinor
Ruby banking system program
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.accessSolution
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
-
I would not assign an instance variable in
-
The
-
I would make
-
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
endContext
StackExchange Code Review Q#162405, answer score: 5
Revisions (0)
No revisions yet.