patternrubyMinor
ATM program with loan section
Viewed 0 times
sectionwithprogramloanatm
Problem
I have spent a few days coding an ATM. What I'm looking for is some critique of my work. Is there anything I can do better here?
The entire project has 9 files and one class, a checking account and savings accounts. It also has a section for loans (thought it was different). Any advice, or input would be greatly appreciated.
To copy this from github the link is: https://github.com/13aal/ATM/tree/master
To make it run use run.rb
main.rb source:
```
#!/usr/bin/env ruby
################
#ATM Rewrite
#
#Creator Lost Bam
#
#11/19/15
##################
require_relative 'checking.rb'
require_relative 'savings.rb'
require_relative 'transfer.rb'
require_relative 'loan.rb'
require_relative './ready/exit.rb'
require_relative './ready/redirect.rb'
class ATM
attr_accessor :name, :checking_account, :savings_account, :pin_number, :transfer, :loan
def initialize( name, checking_account, savings_account, pin_number, credit_score )
@name = name
@checking_account = checking_account
@savings_account = savings_account
@pin_number = pin_number
@credit_score = credit_score
end
def pin
x = 3
while (x > 0) do
puts "Enter PIN(#{x} attempts left):"
pin_num = gets.chomp
case pin_num.to_i
when @pin_number
menu
else
puts "Invalid PIN"
x -= 1
bad_pin if x == 0
end
end
end
def menu
puts /, ' ')
>
>Welcome #{name} thank you for choosing Bank of Bam.
>You may choose from the list below of what you would like to do
>For checking inquiries press '1'
>For savings account information press '2'
>T
The entire project has 9 files and one class, a checking account and savings accounts. It also has a section for loans (thought it was different). Any advice, or input would be greatly appreciated.
To copy this from github the link is: https://github.com/13aal/ATM/tree/master
To make it run use run.rb
main.rb source:
```
#!/usr/bin/env ruby
################
#ATM Rewrite
#
#Creator Lost Bam
#
#11/19/15
##################
require_relative 'checking.rb'
require_relative 'savings.rb'
require_relative 'transfer.rb'
require_relative 'loan.rb'
require_relative './ready/exit.rb'
require_relative './ready/redirect.rb'
class ATM
attr_accessor :name, :checking_account, :savings_account, :pin_number, :transfer, :loan
def initialize( name, checking_account, savings_account, pin_number, credit_score )
@name = name
@checking_account = checking_account
@savings_account = savings_account
@pin_number = pin_number
@credit_score = credit_score
end
def pin
x = 3
while (x > 0) do
puts "Enter PIN(#{x} attempts left):"
pin_num = gets.chomp
case pin_num.to_i
when @pin_number
menu
else
puts "Invalid PIN"
x -= 1
bad_pin if x == 0
end
end
end
def menu
puts /, ' ')
>
>Welcome #{name} thank you for choosing Bank of Bam.
>You may choose from the list below of what you would like to do
>For checking inquiries press '1'
>For savings account information press '2'
>T
Solution
User friendliness
Entering numbers does not feel user-friendly, allowing letters is much more intuitive and just as easy to implement:
and then:
Avoid
Ruby is a high-level language, where
Should be written as:
The second version is not only shorter, but also easier to read.
Avoid writing repetitive things by hand
You write:
But you are a programmer! Programming was invented to avoid repetitive tasks!
This way you can be sure that no value is being left out and can easily modify the
Bug: decimal loss
If you convert amount to integer you will lose all the decimals!
This is not what you want as you then call
Magic numbers
A number is named magic when it appears randomly in the code with no specification as to why it has the value it has.
You should give them a name to allow easier code reading and future changes:
Names of constants are
You give a lot of output and ask for a lot of input, I suggest a function:
You can then simplify your code, for example:
Other functions may be similarly cleaned up.
Replace comments with code
May be written as:
Because the compiler / interpreter can automatically check code for correctness but not comments.
>Deposit funds '1'
>Make a Withdrawl '2'
>Transfer funds '3'
>Go back '4'Entering numbers does not feel user-friendly, allowing letters is much more intuitive and just as easy to implement:
>[D]eposit funds 'D'
>[M]ake a Withdrawl 'M'
>[T]ransfer funds 'T'
>[B]ack 'B'and then:
case gets.chomp.downcase.first
when 'd'
deposit
when 'm'
...first is there to allow deposit or make to be valid inputs.Avoid
while overuseRuby is a high-level language, where
while has a limited use. Any-time you use while think twice.x = 3
while (x > 0) do
...
x -= 1Should be written as:
3.downto(1) do |x|
...The second version is not only shorter, but also easier to read.
Avoid writing repetitive things by hand
You write:
loan = %w(100 200 300 400 500 600 700 800 900 1000 1100 1200 1300 1400 1500 1600 1700 1800 1900 2000 2100 2200 2300 2400 2500 2600 2700 2800 2900 3000 3100 3200 3300 3400 3500 3600 3700 3800 3900 4000 4100 4200 4300 4400 4500 4600 4700 4800 4900 5000 5100 5200 5300 5400 5500 5600 5700 5800 5900 6000 6100 6200 6300 6400 6500 6600 6700 6800 6900 7000 7100 7200 7300 7400 7500 7600 7700 7800 7900 8000 8100 8200 8300 8400 8500 8600 8700 8800 8900 9000 9100 9200 9300 9400 9500 9600 9700 9800 9900 10000)But you are a programmer! Programming was invented to avoid repetitive tasks!
loan = (1..100).map{|x| (x*100).to_s}This way you can be sure that no value is being left out and can easily modify the
loan list.Bug: decimal loss
amount = gets.chomp.to_i
@checking_account += amount.to_fIf you convert amount to integer you will lose all the decimals!
>> "4141.3".to_i
=> 4141This is not what you want as you then call
to_fMagic numbers
A number is named magic when it appears randomly in the code with no specification as to why it has the value it has.
You should give them a name to allow easier code reading and future changes:
CHECK_COST = 5.99
MINIMAL_MONEY_FOR_LOAN = 640
PIN_ENTER_ATTEMPTS = 3Names of constants are
ALL_CAPITAL by convention.prompt function for user inter interactionYou give a lot of output and ask for a lot of input, I suggest a function:
def prompt(text)
puts(text)
gets.chomp
endYou can then simplify your code, for example:
def deposit_checking
@checking_account += prompt("Enter amount to deposit:").to_f
puts "Your new balance is #{@checking_account}"
if prompt("Would you like make another deposit?") == /yes/i
deposit
else
redirect
end
endOther functions may be similarly cleaned up.
Replace comments with code
my_acc = ATM.new("Thomas Park", 5000, 10000, 7856, 640)
my_acc.pin #Checking account
#savings account
#PIN
#credit scoreMay be written as:
my_acc = ATM.new( name = "Thomas Park",
checking_account = 5000,
savings_account = 10000,
pin_number = 7856,
credit_score = 640 )Because the compiler / interpreter can automatically check code for correctness but not comments.
Code Snippets
>Deposit funds '1'
>Make a Withdrawl '2'
>Transfer funds '3'
>Go back '4'>[D]eposit funds 'D'
>[M]ake a Withdrawl 'M'
>[T]ransfer funds 'T'
>[B]ack 'B'case gets.chomp.downcase.first
when 'd'
deposit
when 'm'
...x = 3
while (x > 0) do
...
x -= 13.downto(1) do |x|
...Context
StackExchange Code Review Q#111631, answer score: 6
Revisions (0)
No revisions yet.