HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Ruby infixed math parser

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
infixedparsermathruby

Problem

It only does expressions with 2 operands yet, but I'm wondering if there are any ways I can improve this:

# infix.rb: parse infix-operated math expressions

class String
  def is_number?
    true if Float(self) rescue false
  end
end

class InfixParser
  @operations = [ '/', '*', '+', '-' ]

  testString = '(24 + 6) / 10 * 3'
  shouldEqual = 9

  def parseExpression(expr) #TODO: implement parsing of entire expressions

  end

  def self.parseChunk(chunk)
    chunk = chunk.gsub ' ', ''

    if not (chunk.include?('/') or chunk.include?('*') \
        or chunk.include?('+') or chunk.include?('-'))
        puts "error: no operations in chunk: #{chunk}"
        exit
    end

    firstNumber = ''
    secondNumber = ''

    i = 0
    currentChar = ''
    while not @operations.include? currentChar
      currentChar = chunk[i]
      if not currentChar.is_number? and not @operations.include? currentChar \
         and currentChar != '.'
        puts "error: non-numerical digit in chunk: #{currentChar}"
        exit
      end
      firstNumber += currentChar
      i += 1
    end
    firstNumber = firstNumber[0 .. -2]

    for c in i-1 .. chunk.length-1
      if not chunk[c].is_number? and not @operations.include? currentChar \
         and currentChar != '.'
        puts "error: non-numerical digit in chunk: #{currentChar}"
        exit
      end
      secondNumber += chunk[c]
    end
    secondNumber = secondNumber[1 .. secondNumber.length - 1]

    if chunk[i - 1] == '/'
      return Float(firstNumber) / Float(secondNumber)
    elsif chunk[i - 1] == '*'
      return Float(firstNumber) * Float(secondNumber)
    elsif chunk[i - 1] == '+'
      return Float(firstNumber) + Float(secondNumber)
    elsif chunk[i - 1] == '-'
      return Float(firstNumber) - Float(secondNumber)
    else
      puts "error: invalid operator in chunk: #{chunk}"
    end

  end

end

Solution

Your parseChunk function is not really parsing an expression — it is also evaluating it. Its naming style is also inconsistent with is_number?. I think that eval_binary_expr would be a more appropriate name.

Augmenting the String class with an is_number? method is something to avoid doing. That kind of patching increases the chances of breakage when your code coexists with other code in the same program.

In functions that are meant to be used by other code, calling exit is not an appropriate way to handle errors. You should raise exceptions instead, so that the caller can handle the error in whatever way it chooses.

You appear to be working much too hard to parse the binary expression. I think that this would accomplish more or less the same thing:

def eval_binary_expr(expr)
  l_operand, op, r_operand = expr.partition(%r{[/*+-]})

  if op.empty?
    raise ArgumentError, "Invalid operation or no operation in expression: #{expr}"
  end
  l_operand, r_operand = Float(l_operand), Float(r_operand)

  case op
    when '/' then l_operand / r_operand
    when '*' then l_operand * r_operand
    when '+' then l_operand + r_operand
    when '-' then l_operand - r_operand
  end
end


The solution above makes no attempt to handle negative numbers correctly. As with your original solution, it fails when the first operand is negative.

Code Snippets

def eval_binary_expr(expr)
  l_operand, op, r_operand = expr.partition(%r{[/*+-]})

  if op.empty?
    raise ArgumentError, "Invalid operation or no operation in expression: #{expr}"
  end
  l_operand, r_operand = Float(l_operand), Float(r_operand)

  case op
    when '/' then l_operand / r_operand
    when '*' then l_operand * r_operand
    when '+' then l_operand + r_operand
    when '-' then l_operand - r_operand
  end
end

Context

StackExchange Code Review Q#68691, answer score: 2

Revisions (0)

No revisions yet.