Refactoring a simple Ruby CLI program

Question

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

• akostadinov: it's very strange you're using instance_eval. That's not how a regular program should work and is not needed in your code.
• Jikku Jose: I know its not very common; but it felt more readable like this; rather than calling the methods via an instance and having to repeat it 4 times.

Answer 1

It's very strange you're using 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
end

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

• Jikku Jose: Yes, it felt bad to me too. I feel like an idiot to not think of creating a method like workflow!!
• akostadinov: @JikkuJose, well, if we all didn't do stupid things at times, there wouldn't be any need for reviews, right? :)
• Jikku Jose: Do you see anything other than that?
• akostadinov: @JikkuJose, I didn't spot anything else. As code looks reasonable.