Ruby infixed math parser

Question

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

Answer 1

You've indicated that you prefer not to use regular expressions. In that case, you're still working too hard. In that case, you should look for a library function that does the job, such as the standard scanf library.

require 'scanf'
def eval_binary_expr(expr)
 l_operand, op, r_operand = expr.scanf('%f %c %f')
 if l_operand.nil?
 raise ArgumentError, "Missing or invalid left operand"
 end
 begin
 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
 else raise ArgumentError, "Missing or invalid operator"
 end
 rescue TypeError
 raise ArgumentError, "Missing or invalid right operand"
 end
end

This implementation is better than the original, in that it can handle negative operands and explicitly positive operands. It also examines the string from left to right, which makes it easier to understand its behaviour.

• joshhartigan: thank you. I'm new to ruby, so learning more about things in the standard library is helpful to me.

Answer 2

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.

• joshhartigan: This is all helpful , except I don't want to use regexes - is my method too long even still?
• 200_success: I've posted a second answer to address that.