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
Could you see any "repetition" in your code?
You can begin by trying to trim them out.
#send will help you out.
10 + 2 #=> 12
10.send("+", 2) #=> 12
Abinoam Jr.
···
On Sat, Feb 15, 2014 at 5:36 PM, Zap Zap <lists@ruby-forum.com> wrote:
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
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
calculate %w{5 * 7 + 9 / 3 * 3}
Hope this helps,
Jesus.
···
On Sat, Feb 15, 2014 at 9:36 PM, Zap Zap <lists@ruby-forum.com> wrote:
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!
···
On Sun, Feb 16, 2014 at 4:59 AM, Abinoam Jr. <abinoam@gmail.com> wrote:
Dear "Zap Zap",
Could you see any "repetition" in your code?
You can begin by trying to trim them out.
#send will help you out.
10 + 2 #=> 12
10.send("+", 2) #=> 12
Abinoam Jr.
On Sat, Feb 15, 2014 at 5:36 PM, Zap Zap <lists@ruby-forum.com> wrote:
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
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, Jesús Gabriel y Galán <jgabrielygalan@gmail.com> wrote:
On Sat, Feb 15, 2014 at 9:36 PM, Zap Zap <lists@ruby-forum.com> wrote:
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
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
Also to add to tamouse's suggestion implement parens based precedence into
your parser.
···
On Sun, Feb 16, 2014 at 9:23 AM, tamouse pontiki <tamouse.lists@gmail.com>wrote:
On Sun, Feb 16, 2014 at 4:59 AM, Abinoam Jr. <abinoam@gmail.com> wrote:
> Dear "Zap Zap",
>
> Could you see any "repetition" in your code?
> You can begin by trying to trim them out.
>
> #send will help you out.
>
> 10 + 2 #=> 12
> 10.send("+", 2) #=> 12
>
> Abinoam Jr.
>
> On Sat, Feb 15, 2014 at 5:36 PM, Zap Zap <lists@ruby-forum.com> wrote:
>> 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
>>
>> calculator(['5','*','7','+','9','/','3','*','3'])
>>
>> --
>> Posted via http://www.ruby-forum.com/\.
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!