Problem
I am new to Ruby and would like some help with re-factoring my code.
class Beam
def initialize
puts "Please specify span of beam"
@span = gets.chomp.to_f
puts "How many number of point loads do you want?"
@pointLoadNos = gets.chomp.to_i
@pointLoad = Array.new(@pointLoadNos)
@pointLoadDistFromLeft = Array.new(@pointLoadNos)
@pointLoadDistFromRight = Array.new(@pointLoadNos)
@magnitude = Array.new(@pointLoadNos)
@reaction = Array.new(2,0)
end
def setValues
count = 0
while count < @pointLoadNos
puts "At what dist should point load #{count+1} be placed from left?"
@pointLoadDistFromLeft[count] = gets.chomp.to_f
if @pointLoadDistFromLeft[count] > @span
puts "Dist of Point Load #{count+1} From Left should be less than span length of beam"
else
puts "Magnitude of point load #{count+1} should be?"
@magnitude[count] = gets.chomp.to_f
@pointLoadDistFromRight[count] = (@span - @pointLoadDistFromLeft[count])
count += 1
end
end
end
def calReactions
i = 0
while i < @pointLoadNos
@reaction[0] += (@pointLoadDistFromLeft[i]*@magnitude[i])/@span
@reaction[1] += (@pointLoadDistFromRight[i]*@magnitude[i])/@span
i += 1
end
puts "Reaction at Left: #{@reaction[0]}"
puts "Reaction at Left: #{@reaction[1]}"
end
end
beam = Beam.new
beam.setValues
beam.calReactions
Solution
First of all, you should not get user input from inside your Beam class. This ties it to the console and makes automated testing more difficult. A layered architecture is more suitable :
# The Beam class is only concerned by business logic.
# Notice how the logic and workflow are clear and readable,
# and how easy you it would be to write tests for it.
#
class Beam
attr_reader :span
def initialize( span )
@span = span.to_f
@load_points = {}
end
def add_load_point( distance, magnitude )
distance, magnitude = distance.to_f, magnitude.to_f
if distance < 0 || distance > span || @load_points[distance]
raise ArgumentError, 'invalid load point'
end
@load_points[distance] = magnitude
end
def reactions
@load_points.inject( left: 0, right: 0 ) do |hash, (distance, magnitude)|
hash[:left] += distance * magnitude / span
hash[:right] += (span - distance) * magnitude / span
hash
end
end
end
# this module is an interface between your class and the user
# via the console. Notice that this module (could be a class,
# does not matter) is only concerned about asking questions,
# adapting user input to feed the Beam class interface, and
# formatting output.
#
module BeamConsole
# here we chose to move all questions to the user to methods,
# and name these methods with an exclamation mark at the end
#
def self.invoke
beam = Beam.new( span? )
begin
beam.add_load_point( distance?, magnitude? )
rescue ArgumentError => e
puts "Invalid parameters for load point. Please retry." ; retry
end while more?
format_reactions( beam.reactions )
end
private #===================================================================
# adapted from @nakilon's answer, tail-recursive variant
def self.prompt( msg )
output = gets.strip!
return output unless output.empty?
prompt( msg )
end
def self.span?
prompt( 'Please specify span of beam' ).to_f
end
def self.more?
answer = prompt( 'Do you want to add points ? (y/n)' )
answer = prompt( 'please answer by y or n' ) unless answer =~ /^[y|n]$/
answer == 'y'
end
def self.distance?
prompt( 'At what distance should the point be placed from left?' ).to_f
end
def self.magnitude?
prompt( 'Magnitude should be?' ).to_f
end
def self.format_reactions( reactions )
puts "Reaction on the left is : %.4f" % reactions[:left]
puts "Reaction on the right is : %.4f" % reactions[:right]
end
end
This may seem overkill, but separating concerns is the core of OOP. With this approach, you’ll end up with a Beam class devoid of any responsability concerning user interaction, and that’s a good thing : you want your class to perform business logic before anything else.
This way, your code will be more readable (no logic riddled with prompts), so more maintainable, but also more reusable. Imagine that one day you want to allow users to perform calculations from a web page : you could not reuse your Beam class if it is tied to the console, but you could if you separated responsibilities.
I would write something like:
class Beam
def prompt msg
# a bit kung-fu, sorry
puts msg while gets.strip!.empty?
yield $_
end
def initialize
@span = prompt "Please specify span of beam", &:to_f
@pointLoadNos = prompt "How many number of point loads do you want?", &:to_i
@pointLoad = []
@pointLoadDistFromLeft = []
@pointLoadDistFromRight = []
@magnitude = []
end
def setValues
@pointLoadNos.times do |count|
while 0 > @pointLoadDistFromRight[count] = @span - @pointLoadDistFromLeft[count] =
prompt("At what dist should point load #{count + 1} be placed from left?", &:to_f)
puts "Dist of Point Load #{count + 1} From Left should be less than span length of beam"
end
@magnitude[count] = prompt "Magnitude of point load #{count + 1} should be?", &:to_f
end
end
def calReactions
@reaction = [0, 0]
@magnitude.zip(@pointLoadDistFromLeft, @pointLoadDistFromRight) do |mag, left, right|
@reaction[0] += (left * mag) / @span
@reaction[1] += (right * mag) / @span
end
puts "Reaction at Left: #{@reaction[0]}"
puts "Reaction at Right: #{@reaction[1]}"
end
end
beam = Beam.new
beam.setValues
beam.calReactions
See this and this – when you pass a block, you don’t have to catch it into argument, but can just use Ruby’s keyword yield. Also instead of describing a block as a proc, you can use &:method notation.