Can anybody recommend a better way of creating a calculator than the one
I have attempted below. I feel my method is inefficient and can be
optimized.
def calculator(s)
while true
s.each_with_index do |x,index|
case x
when ‘’
result = s[index-1].to_f * s[index+1].to_f
s[index-1] = result
s.slice!(index,2)
break
when ‘/’
result = s[index-1].to_f / s[index+1].to_f
s[index-1] = result
s.slice!(index,2)
break
end
end
break if !s.include?(’’) and !s.include?(’/’)
end
while true
s.each_with_index do |x,index|
case x
when ‘+’
result = s[index-1].to_f + s[index+1].to_f
s[index-1] = result
s.slice!(index,2)
break
when ‘-’
result = s[index-1].to_f - s[index+1].to_f
s[index-1] = result
s.slice!(index,2)
break
end
end
break if !s.include?(’+’) and !s.include?(’-’)
end
puts s
end
Abinoam’s advice is great: first reduce repetition.
If you are up for a bit of a challenge, you might look at how you can
make your calculator “parse” your in-fix notation array of tokens into
a pre-fix structure, that you can then reduce to the result. Good fun!
Apart from others’ comments which are interesting, I wanted to show
how I’d do a simple evaluator, which is similar to your approach but
using a temporary array to avoid so much slicing:
def is_high_precedence_operator o
o == ‘*’ || o == ‘/’
end
def calculate tokens
temp = []
tokens.each do |t|
if is_high_precedence_operator(temp.last)
operator = temp.pop
first = temp.pop.to_i
temp.push first.send(operator, t.to_i)
else
temp.push t
end
end
total = temp.first.to_i
temp[1…-1].each_cons(2) do |op,second|
total = total.send(op,second.to_i)
end
total
end
Basically I just tried to trim the repetition maintaining (at least
partially) your original approach.
def calculator(s)
response = s.dup
[[’*’, ‘/’], [’+’, ‘-’]].each do |ops|
ix = 0
while s[ix]
if ops.include? s[ix+1]
n1, op, n2 = s[ix,3]
s[ix,3] = n1.to_f.send(op, n2.to_f)
else
ix += 2
end
end
end
s[0]
end
Abinoam Jr.
On Mon, Feb 17, 2014 at 5:34 AM, Jess Gabriel y Galn
Try this one.
It’s not a perfect example, but, perhaps you can improve it…
def is_number? expr
return false if expr.nil?
expr = “#{expr}” # we need this condition in case the
expr is a number
expr.match /^(\d+|\d+.\d+)$/ # since match() is defined only for
strings
end
def calc(expr)
return expr.to_i if is_number? expr
expr.gsub!(" “,”") # clean the string from whitespaces
pay attention to the order: + and - should come before * and /