# Ruby banking system program

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

class Account

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

• this is quite lovely. i really don't have much to add to it. i guess you only want the access method to be publicly available. the rest you would probably leave as private methods - because they might change. but this is pretty sweet IMO and i can't comment unless there are further changes to the requirements. – BKSpurgeon May 3 '17 at 14:06

I do have one comment on some low-level stuff: Use a string for the PIN instead of an int. When it's an integer, you can't have leading zeros, so if James Bond's PIN is 007, you could enter 7, 07, or 0000000007, and all would be equally valid. That's no good (and James Bond - already the world's worst "secret" agent is a fool for having that PIN anyway).

I'd also suggest making the PIN a constructor argument - otherwise everyone in your bank will have the same hard-coded PIN. Not exactly secure.

As for structure: Your Account class isn't only an account - it's a complete ATM, too. User interface (the command line in this case), is mixed in with the business logic of managing the balance.

I'd suggest separating those concerns. One would be the Account model/class that holds the information, and performs actions on said information, and the other would encapsulate the user interface.

Right now, if you decide that you want to do web banking too - not just command line banking - you'd run into trouble. You'd have to make a brand new Account class to handle it. But that wouldn't make sense, since mr Bond only has (or should only have) one account - not different ones depending on how he's accessing them.

I'd do something like this:

class Account

def initialize(name, pin, balance)
@name = name
@pin = pin # TODO: Check that it's a valid PIN and not just blank or something
@balance = balance
end

def valid_pin?(pin)
@pin == pin
end

def withdraw(amount)
# subtract amount from balance or raise error if balance
# isn't large enough (or amount is negative)
end

def deposit(amount)
# add amount to balance or raise error if amount is negative
end
end

class Atm
def initialize(account)
@account = account
end

# ...
end


The Atm class handles all the user interaction much like now, but delegates the actual withdrawal/depositing and PIN checking to the account. How you structure it from here is up to you, but the basic point is that the account is "just" an account.

• 1. Nice one, though it doesn't work right after i've replaced int with string for pin input. I'll continue trying. – edvard_munch May 4 '17 at 13:45
• 2. I'll definitely make some structural changes, got your point. – edvard_munch May 4 '17 at 13:52
• hello, can you give me the hint please, how to make these two classes work together without inheritance or using "Account" as a module? In your example you are passing the amount as an argument for two different methods. It is supposed to get the actual amount from the user input in the Atm class? I didn't ask right away because I thought I could figure this out myself, with the help of mighty google. Found some examples of simple ATM programs in Ruby and Python (since it very similar and more popular now). – edvard_munch May 16 '17 at 6:47
• Got some interesting new ideas, but nothing to help me figure out how to do this in the way that you mean. And can you tell me more about problems I'd run into if i'll use my current structure for web app? What kind of problems? Sorry but as a beginner I don't see it clearly :). – edvard_munch May 16 '17 at 7:07
• @Flynn84 Apologies, but it seems I won't have time to add much to my answer for the next couple days. But to quickly answer you question about using it for a web app: Your current code expect input from a terminal (asking for the the PIN, and you typing it in, etc.), but if you're running things on a server, people would be using it through a browser: There's no terminal. Hence why I said to separate user interface (terminal in this case, website in another) from core functionality (the account) – Flambino May 18 '17 at 8:13

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!

• 1. I've read on stackowerflow probably week or two ago that it's better to have all private methods at the top, so I have them there. Except initialize, that always should be at the top? I hope I understand it properly that there is need to put "private" before every method you want to be private? They all private before I put the "public" word? I've checked - it worked in this program. – edvard_munch May 4 '17 at 11:50
• 2. Yeah, thanks. That is obvious, I've never even paid attention to these methods since they worked properly from the very beginning. – edvard_munch May 4 '17 at 11:53
• 3. Other suggestions are useful too, thanks. – edvard_munch May 4 '17 at 13:53
• Nice answer +1. Took the liberty of editing just to format the code block (it was being rendered as regular text) – Flambino May 4 '17 at 14:02
• wrt To private positioning, all the projects I have worked on have them at the bottom and that is my preference but it doesn't really matter as long as you are consistent within a project. – Marc Rohloff May 4 '17 at 16:09

There are few guidelines which I would follow while coding in Ruby:

• Indentation in Ruby is 2 spaces.

• In any program or project, improve understanding of the project by using proper documentation. Like Java has Javadoc for documentation, Ruby has Tomdoc, which helps in providing documentation to project.

• Rubocop is a gem in Ruby, which can be installed on the local system, and from project location, when we run rubocop gives warning on style.

• attr_reader method is used to read variables; attr_writer method is used to write variables. To both read and write, we use attr_accessor. In your case balance is read and written, so we can use attr_accessor.

• Usually in Banking sector, the menu option repeats after transaction. Even in your case that can be achieved, by providing wcase statement in the while loop and the menu can have another option: exit :e.

• Pin is unique for each user; we can prompt user to enter pin rather than hardcoding it.

• We can use proper exception Handling. Instead of just printing the error message we can raise the error and rescue the error message from calling application.