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

ATM program with loan section

Submitted by: @import:stackexchange-codereview··
0
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

Solution

User friendliness

>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 overuse

Ruby 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 -= 1


Should 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_f


If you convert amount to integer you will lose all the decimals!

>> "4141.3".to_i
=> 4141


This is not what you want as you then call to_f

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:

CHECK_COST = 5.99
MINIMAL_MONEY_FOR_LOAN = 640
PIN_ENTER_ATTEMPTS = 3


Names of constants are ALL_CAPITAL by convention.

prompt function for user inter interaction

You give a lot of output and ask for a lot of input, I suggest a function:

def prompt(text)
   puts(text)
   gets.chomp
end


You 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
end


Other 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 score


May 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 -= 1
3.downto(1) do |x|
     ...

Context

StackExchange Code Review Q#111631, answer score: 6

Revisions (0)

No revisions yet.