# Bank ATM machine in Ruby

I've made a simple implementation of a bank ATM machine in pure Ruby. I'd like to hear some recommendations to make my code more idiomatic.

### atm.rb

This is the startup file. I didn't use a class and I'm getting the user input using gets.chomp and a case statement for the flow control.

require_relative 'account'

account = Account.new

loop do
puts "What you want to do?\n1- Deposit\n2- Withdraw\n3- Check balance\n4- Check statement\n5- Exit"
option = gets.chomp
case option
when '1'
puts "How many dollars do you want to deposit?"
amount = gets.chomp.to_f
account.deposit(amount)
when '2'
puts "How many dollars do you want to withdraw?"
amount = gets.chomp.to_f
account.withdraw(amount)
when '3'
puts "You have #{account.balance} USD."
when '4'
puts account.statement.join("\n")
when '5'
break
else
"Wrong option. Try again."
end
end


### account.rb

I used an array to store the transactions. To check the balance, I'm iterating through the array using the inject function.

require_relative 'transaction'

class Account
def initialize
@transactions = []
end

def balance
@transactions.inject(0) { |sum, transaction| sum + transaction.value }
end

def deposit(amount)
end

def withdraw(amount)
end

def statement
@transactions.map { |transaction| transaction.to_s }
end

private
attr_accessor :transactions

@transactions << Transaction.new(amount, type)
end
end


### transaction.rb

This is the transaction class. I don't know if it's ok to validate the constructor parameters the way I did.

class Transaction
attr_accessor :amount, :type, :timestamp

def initialize(amount, type)
raise 'Invalid type' unless POSSIBLE_TYPES.include? type
@amount = amount
@type = type
@timestamp = Time.now.getutc
end

def value
return @amount if type == :deposit
-@amount
end

def to_s
"#{@timestamp} - #{type} - #{amount} USD"
end

private
POSSIBLE_TYPES = [:deposit, :withdraw]
end


I'm aware that I'm not validating the user input type or value, I'm ok with that.

See it running in repl.it.

• By the way: it is really great that you recognized the Transaction abstraction. In almost all OO tutorials, bank accounts are used, but almost always it is taught that a transaction should be an operation and the balance should be data. When you do this, you run into a lot of trouble with synchronization, once you have concurrent access to your bank accounts. By modeling a transaction as data and the balance as an operation (a fold of the transaction log), you side-step a lot of those problems later on. Kudos for that! – Jörg W Mittag Dec 25 '17 at 18:34

require_relative 'account'

account = Account.new

loop do
# This is totally opinion, but I think doing this by joining a string array looks neater.
# puts "What you want to do?\n1- Deposit\n2- Withdraw\n3- Check balance\n4- Check statement\n5- Exit"
puts "What you want to do?"
options = [
"1- Deposit",
"2- Withdraw",
"3- Check balance",
"4- Check statement",
"5- Exit"
]
puts options.join("\n")
# These two lines can be condensed into one
# option = gets.chomp
# case option
case gets.chomp
when '1'
puts "How many dollars do you want to deposit?"
# You could merge these lines into account.deposit(get.chomp.to_f)
amount = gets.chomp.to_f
account.deposit(amount)
when '2'
puts "How many dollars do you want to withdraw?"
# You could merge these lines into account.withdraw(get.chomp.to_f)
amount = gets.chomp.to_f
account.withdraw(amount)
when '3'
puts "You have #{account.balance} USD."
when '4'
puts account.statement.join("\n")
when '5'
break
else
"Wrong option. Try again."
end
end


This is generally fine, just a couple things that I'd prefer for readability. I have larger ideas for structure for the other two files.

For the Transaction class, you really don't need the @type instance variable if it can only be :deposit or :withdraw. You can really just make that information be held in the sign of @amount. If @amount > 0, it's a deposit, and if @amount < 0 it's a withdraw. So, with that in mind, here's my revised version:

class Transaction
# This was an attr_accessor. These attributes don't need to be editable, only readable.

def initialize(amount)
@amount = amount.to_f
@timestamp = Time.now.getutc
end

def to_s
# @amount is an instance variable, and so I added the "@" before it
# You don't really need type here, but if you want it, use the next line:
type = @amount > 0 ? "deposit" : "withdraw"
"#{@timestamp} - #{type} - #{@amount} USD"
end
end


And now, we move to the final file, account.rb:

require_relative 'transaction'

class Account
def initialize
@transactions = []
end

def balance
# With our modifications to Transaction, we can do this more simply
# @transactions.inject(0) { |sum, transaction| sum + transaction.value }
@transactions.map(&:amount).sum
end

# In the next 2 methods, we don't to pass type anymore.
# We can also remove the private add_transaction method, because it doesn't really serve any purpose. If you wanted to modify the transaction creation process, you should do it in Transaction#initialize.
# I've also added optional type handling, for fun

def deposit(amount)
@transactions << Transaction.new(amount)
end

def withdraw(amount)
@transactions << Transaction.new(-amount)
end

def statement
# This can be simplified with the map(&:to_s) syntax
# @transactions.map { |transaction| transaction.to_s }
@transactions.map(&:to_s)
# You may also want to make the call to #join here, instead of in the main loop, because a statement sounds like it should be a string.
end
end


Edit:

You could also get rid of the Transaction#to_s method and instead modify the Account#statement method to be:

class Account
def statement
@transactions.map do |t|
type = t.amount > 0 ? "deposit" : "withdraw"
"#{t.timestamp} - #{type} - #{t.amount} USD"
# Personally, I like the extra line for readability, but
# you could also write it as:
# "#{t.timestamp} - #{t.amount > 0 ? 'deposit' : 'withdraw'} - #{t.amount} USD"
end
end
end

• No problem! If you have any questions about ruby, feel free to ping me in the ruby chat room – thesecretmaster Dec 18 '17 at 2:49
• I like the way you changed the Transaction#initialize. In my implementation I was raising an exception when the argument passed was invalid. Is that the way to go? – Vinicius Brasil Dec 18 '17 at 2:56
• That would also work. By calling #to_f on amount, it will throw an error for anything that does not respond to the #to_f method. For example, if you passed an array, you'd get an error. But anything that can be coerced to be a float will be. – thesecretmaster Dec 18 '17 at 2:58
• Really great improvements. I'll note that the only place you have a local variable is in transaction.to_s. The perfectionist in me would extract type to a private method, or nix it altogether. – Kori John Roys Dec 28 '17 at 22:38
• @KoriJohnRoys Hmmm... maybe. I feel like since it's only used in one place, it's better to just use a local variable. And if you're looking at that method anyways, I feel like the formatting should be mixed into Account#statement instead of Transaction#to_s, like it is. Or at least renamed. Really, a Transaction can't be coerced into a string, it has to be formatted to be one. – thesecretmaster Dec 29 '17 at 0:40