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.
2 Answers 2
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 output0
.There is no need for
getPriorizedOperators
to be a function. Just declare the list as a variable inoperator.py
, then import it incalculator.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.
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 tooperatorArrayIndex
:def calculate(equation, operatorArrayIndex=0):
You rely on the fact that a
return
inside afinally
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 anexcept
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.
Explore related questions
See similar questions with these tags.