Basic arithmetic calculator

Question

The calculator supports the basic operators (Add,Substract,Divide,Multiply).

At the moment,

• It only works with positive integers
• I do not validate my user input
• My division rounds up if it isn't an integer

This is a work in progress. It is my first Python application so I want to get feedback before I keep on working on it.

Here is an example of how my calculator parses/processes an equation :

Given 3+3+9/3-4*2

• Computing multiplications : 3+3+9/3-8 (Note that 4*2 is now 8)

• Then divisions : 3+3+3-8 (9/3 = 3)

• Then additions : 9-8 (3+3+3 = 9)

• Finally substractions : 1 (9-8 = 1)

import operator

priorizedOperators = operator.getPriorizedOperators()

def calculate(args):
    return _calculate(args,0)

def _calculate(equation, operatorArrayIndex):

    if len(priorizedOperators) == operatorArrayIndex:
        return equation;

    arithmeticOperator = priorizedOperators[operatorArrayIndex]
    copiedEquation = equation

    try:
        operatorLocation = copiedEquation.index(arithmeticOperator.getStringSign())

        while True:
            leftBound = _findLeftBound(equation,operatorLocation)
            rightBound = _findRightBound(equation,operatorLocation)

            leftValue = int(copiedEquation[leftBound:operatorLocation])
            rightValue = int(copiedEquation[operatorLocation+1:rightBound])

            temp = arithmeticOperator.operate(leftValue,rightValue)

            #Replaces the part of the equation we just computed by the result
            copiedEquation = copiedEquation[:leftBound] + str(temp) + copiedEquation[rightBound:]

            #This will throw an exception if index doesn't find the operator, which ends the while loop
            operatorLocation = copiedEquation.index(arithmeticOperator.getStringSign())
    finally:
        return _calculate(copiedEquation, operatorArrayIndex + 1)

def _findLeftBound(equation ,operatorIndex):
    for leftBoundSearchIndex in reversed(range(0,operatorIndex)):
        if not _isNumber(equation[leftBoundSearchIndex]):
            return leftBoundSearchIndex + 1

    return 0

def _findRightBound(equation, operatorIndex):
    for rightBoundSearchIndex in range(operatorIndex+1, len(equation)):
        if not _isNumber(equation[rightBoundSearchIndex]):
            return rightBoundSearchIndex

    return len(equation)

def _isNumber(char):
    return ord("0") <= ord(char) <= ord("9")

def main():
    equation = input("Enter equation : ")
    print(calculate(equation))

if __name__ == "__main__": main()

Content of the operator.py

class PlusOperator():
    def getStringSign(self):
        return "+"

    def operate(self,left,right):
        return left + right

class MinusOperator():
    def getStringSign(self):
        return "-"

    def operate(self,left,right):
        return left - right

class MultiplyOperator():
    def getStringSign(self):
        return "*"

    def operate(self,left,right):
        return left * right

class DivideOperator():
    def getStringSign(self):
        return "/"

    def operate(self,left,right):
        return left // right

def getPriorizedOperators():
    return [MultiplyOperator(),DivideOperator(),PlusOperator(),MinusOperator()]

I'm looking to know if my python is okay and if my algorithm makes sense.

Answer 1

A few quick comments:

• Read PEP 8, the Python style guide. In particular, Python uses lowercase_with_underscore variable names, and put commas after spaces.

• Add some docstrings to your functions, so that it’s clear what purpose they serve in the context of your calculator. This will become useful as you extend the calculator, and you want to change existing function: you may remember what each function is for at the moment, but you may not in six months time.

• Add more comments. Tell me why you’re doing certain operations. Anybody can read the code and see what it’s doing, but why is it doing it?

  For example: after some inspection, I see that you iterate through prioritizedOperators to apply each operation in order (multiplication and division, then subtraction and addition).

• You have no error handling, which means I can enter something silly like 1/0 and get a ZeroDivisionError. You also claim that it only works for positive integers, but the simple examples I tried with negative integers seemed to work as well.

  If you only want it to work with positive integers, then you should spit out any input that isn’t positive.

• You claim that

  My division rounds up if it isn't an integer

  but this doesn’t seem to be true. For example, if I enter 2/3 then I get output 0.

• There is no need for getPriorizedOperators to be a function. Just declare the list as a variable in operator.py, then import it in calculator.py. For example:

  operator.py:

  prioritized_operators =  [
      multiply_operator(),
      divide_operator(),
      plus_operator(),
      minus_operator()
  ]
  

  calculator.py:

  from operator import prioritized_operators
  

• I would rename your _isNumber() function. It only checks if a single character is a numeral, but the name implies I might be able to pass in arbitrary-length strings. Perhaps _is_digit() would be better.

  You can tidy up the function logic a little. Rather than calling out to ord(), you can also compare strings directly for the same result. For example:

  >>> "0" < "5" < "9"
  True
  >>> "0" < "x" < "9"
  False
  

  Or you could just use the builtin function isdigit(), and do away with your function entirely:

  >>> "5".isdigit()
  True
  >>> "x".isdigit()
  False
  

  (Whether you use the latter depends on how much you want to reinvent the wheel.)

• Having two functions called calculate() and _calculate() is prone for confusion. Rename one or both of them to have a more specific name.

Answer 2

In addition to points made by alexwchan:

• operator is a module in the standard library. Use a different name for yours.

• You could combine calculate and _calculate by giving a default value to operatorArrayIndex:

  def calculate(equation, operatorArrayIndex=0):
  

• You rely on the fact that a return inside a finally block causes the exception to be discarded. That is correct, but it means ignoring any exception that may occur, which can hide bugs. Always catch the specific exception you expect with an except clause.

• This calculator does not adhere to the standard order of operations where multiplication has the same precedence as division, and addition the same as subtraction. For example, 1-2+3 evaluates to -4 by this code.