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'])

on 2014-02-15 21:36

on 2014-02-16 12:00

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 2014-02-16 16:23

On Sun, Feb 16, 2014 at 4:59 AM, Abinoam Jr. <abinoam@gmail.com> wrote: > Abinoam Jr. >> when '*' >> end >> when '-' >> >> 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!

on 2014-02-17 07:46

Also to add to tamouse's suggestion implement parens based precedence into your parser.

on 2014-02-17 09:34

On Sat, Feb 15, 2014 at 9:36 PM, Zap Zap <lists@ruby-forum.com> wrote: > s[index-1] = result > end > s[index-1] = result > > -- > Posted via http://www.ruby-forum.com/. 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 2014-02-17 11:20

Dear Zap Zap, 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

on 2014-02-17 17:47

```
Zap Zap wrote in post #1136795:
> Can anybody recommend a better way ....
If 'better' mean less code lines (...) this one :
expr=ARGV.join("")
loop {
expr.gsub!(/(\d+)([\/*])(\d+)/) { $1.to_i.send($2,$3.to_i).to_s}
expr.gsub!(/(\d+)([+-])(\d+)/) { $1.to_i.send($2,$3.to_i).to_s }
(puts expr; break ) if expr =~/^\d+$/
}
```

on 2014-05-07 23:19

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 / # can you figure out why ? arr = expr.split /\+/ return arr.inject(0){|x,y| calc(x) + calc(y) } if arr.size > 1 arr = expr.split /\-/ return arr.inject(0){|x,y| calc(x) - calc(y) } if arr.size > 1 arr = expr.split /\*/ return arr.inject(1){|x,y| calc(x) * calc(y) } if arr.size > 1 arr = expr.split /\// return arr.inject {|x,y| calc(x) / calc(y) } if arr.size > 1 end a = gets puts calc(a)