patternrubyMinor
Refactoring a simple Ruby CLI program
Viewed 0 times
refactoringsimpleprogramcliruby
Problem
I tried to help someone in Stackoverflow with a refactoring exercise. Did many changes to his original code and made a somewhat decent solution (at least in my eyes). Was thinking, whether someone can critique on my implementation.
class Theatre
COST = { running: 3, fixed: 180 }
attr_accessor :number_of_audience, :ticket_price
def revenue
@number_of_audience * @ticket_price
end
def total_cost
COST[:fixed] + (@number_of_audience * COST[:running])
end
def net
revenue - total_cost
end
def profit?
net > 0
end
end
class TheatreCLI
def initialize
@theatre = Theatre.new
end
def seek_number_of_attendes
print 'Number of audience: '
@theatre.number_of_audience = gets.chomp.to_i
end
def seek_ticket_price
print 'Ticket price: '
@theatre.ticket_price = gets.chomp.to_i
end
def print_revenue
puts "Revenue for the theatre is RM #{@theatre.revenue}."
end
def print_profit
message_prefix = @theatre.profit? ? 'Profit made' : 'Loss incurred'
puts "#{message_prefix} #{@theatre.net.abs}."
end
def self.run
TheatreCLI.new.instance_eval do
seek_ticket_price
seek_number_of_attendes
print_revenue
print_profit
end
end
end
TheatreCLI.runSolution
It's very strange you're using
If you want to avoid calling 4 instance methods from a class instance method, then you can create a new instance method like:
Maybe it's not a problem here, but using a lot of
instance_eval. That's not how a regular program should work and is not needed in your code.If you want to avoid calling 4 instance methods from a class instance method, then you can create a new instance method like:
def workflow
seek_ticket_price
seek_number_of_attendes
print_revenue
print_profit
end
def self.run
TheatreCLI.new.workflow
endMaybe it's not a problem here, but using a lot of
instance_eval for saving keystrokes looks like working around a bad API. Also my gut feeling tells me that such practice can lead to unexpected troubles if used outside class body.Code Snippets
def workflow
seek_ticket_price
seek_number_of_attendes
print_revenue
print_profit
end
def self.run
TheatreCLI.new.workflow
endContext
StackExchange Code Review Q#77683, answer score: 2
Revisions (0)
No revisions yet.