Frage

One of the pre-work exercises for Dev Bootcamp is an RPN calculator. I made it work but would like refactoring feedback. Any and all help to make this code cleaner is greatly appreciated.

class RPNCalculator
  def evaluate(rpn)
    a = rpn.split(' ')
    array = a.inject([]) do |array, i|    
      if i =~ /\d+/ 
        array << i.to_i
      else
        b = array.pop(2)
        case 
          when i == "+" then array << b[0] + b[1]  
          when i == '-' then array << b[0] - b[1]  
          when i == '*' then array << b[0] * b[1]  
          when i == '/' then array << b[0] / b[1]  
        end
      end
    end
    p array.pop
  end
end

calc = RPNCalculator.new
calc.evaluate('1 2 +')   # => 3
calc.evaluate('2 5 *')   # => 10
calc.evaluate('50 20 -') # => 30
calc.evaluate('70 10 4 + 5 * -') # => 0  
War es hilfreich?

Lösung

class RPNCalculator
  def evaluate rpn
    array = rpn.split(" ").inject([]) do |array, i|    
      if i =~ /\d+/ 
        array << i.to_i
      else
        b = array.pop(2)
        array << b[0].send(i, b[1])
      end
    end
    p array.pop
  end
end

Andere Tipps

I tend to prefer avoiding case..when in favor of lookup tables. So I'd change your code to:

class RPNCalculator
  def evaluate(rpn)
    a = rpn.split(' ')
    array = a.inject([]) do |array, i|    
      if i =~ /\d+/ 
        array << i.to_i
      else
        array << array.pop(2).reduce(op(i))
      end
    end
    p array.pop
  end

  private

  def op(char)
    {'+'=>:+, '-'=>:-, '/'=>:/, '*'=>:*}[char]
  end
end

I also don't believe you should only be popping off 2 operands. "1 2 3 +" would be valid RPN, evaluating to 6. The entire stack should be reduced. This also avoids the mutation, which is a good thing, as it follows a more functional style.

class RPNCalculator
  def evaluate(rpn)
    a = rpn.split(' ')
    array = a.inject([]) do |array, i|    
      if i =~ /\d+/ 
        [*array, i.to_i]
      else
        [array.reduce(op(i))]
      end
    end
    p array.pop
  end

  private

  def op(char)
    {'+'=>:+, '-'=>:-, '/'=>:/, '*'=>:*}[char]
  end
end

I removed the other mutation here too, by using [*arr, value] instead of actually modifying the array.

Finally, I'd avoid printing directly from your #evaluate method and just return the number. I'd also (again) avoid the mutation:

class RPNCalculator
  def evaluate(rpn)
    a = rpn.split(' ')
    stack = a.inject([]) do |stack, i|    
      if i =~ /\d+/ 
        [*stack, i.to_i]
      else
        [stack.reduce(op(i))]
      end
    end
    stack.last
  end

  private

  def op(char)
    {'+'=>:+, '-'=>:-, '/'=>:/, '*'=>:*}[char]
  end
end

I renamed 'array' to 'stack', since it is a parser stack and is less generic than just array.

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top