# Python PEMDAS calculator [closed]

The goal of this calculator is to evaluate an expression with PEMDAS in mind (no parenthesis yet, adding soon). Although the program works, I want to make it as efficient as possible with the approach I used to attack the mathematical expression.

So the way I decided to approach the problem is to initially organize the expression via my cleanUp() so that it's easier to manipulate/iterate afterwards, for example:

"2+2 -4 * 1 / 3.344 - -2" becomes "2 + 2 - 4 * 1 / 3.344 + 2" which can now be easily split by a space and the calculation process is now a lot easier, but I feel like my cleanUp() function can be a lot more efficient. (I'm still learning Time complexities of built-in functions of Python 3)

Here is my whole code, feel free to comment on any other part of the program, but I'm mostly asking for optimization improvements on the cleanUp() function.

import time

def isNumber(s):
#s must be a non-empty string
#returns True if it's convertible to float, else False
if (len(s)==0 and not s =='') or not isinstance(s, str):
print("type mismatch error: isNumber")
return "type mismatch error: isNumber"
try:
s = float(s)
return True
except ValueError:
#print("type mismatch error: isNumber")
return False

def cleanUp(s):
priorSpace = True
inDecimal = False
ops = ["+","-","*","/","^"]
s = s.replace("+", " + ").replace("-", " - ").replace("*", " * ").replace("/", " / ").replace("^", " ^ ")
st = list(s)
for i,c in enumerate(s):
if (not isNumber(c) and c not in ops) and priorSpace:
if c == "." and not inDecimal:
inDecimal = True
elif c == "." and inDecimal:
st[i] = ""
else:
st[i] = ""
inDecimal = False
elif (not isNumber(c) and c not in ops) and not priorSpace:
if c == "." and not inDecimal:
inDecimal = True
elif c == "." and inDecimal:
st[i] = ""
else:
st[i] = " "
priorSpace = True
inDecimal = False
else:
priorSpace = False
final = ''.join(st).strip().split(' ')
if '-' in final[0]:
del final[0]
final[0] = '-' + final[0]
newNumber, newOpr, oprPos = getNextNumber(final, 0)
if oprPos == None:
return newNumber
cacheNum=newNumber
pos=oprPos+1                #the new current position
opr=newOpr
while pos<(len(final)-1):
newNumber, newOpr, oprPos = getNextNumber(final, pos)
if (newOpr == None):
break
if (not isNumber(newNumber) and newNumber == '-'):
del final[oprPos-1] #BOOKMARK
final[oprPos-1] = '-' + final[oprPos-1]
if (not isNumber(newNumber) and newNumber != '-'):
print("error at line AB", newNumber)
return "error at line AB"
pos=oprPos+1
return (final)

def getNextNumber(expr, pos):
#expr is a given arithmetic formula in string
temp = expr
#pos = start position in expr
#1st returned value = the next number (None if N/A)
#2nd returned value = the next operator (None if N/A)
#3rd retruned value = the next operator position (None if N/A)
ops = ["+","-","*","/"]
if len(expr)==0 or pos<0 or pos>=len(expr) or not isinstance(pos, int):
print("type mismatch error: getNextNumber")
return None, None, "type mismatch error: getNextNumber"
#--- function code starts ---#
res3 = 0
res2 = 0
if (pos+1) < len(temp):
res3 = pos+1
res2 = temp[pos+1]
else:
res3 = None
res2 = None
return ((temp[pos], res2, res3))

#--- function code ends ---#

def exeOpr(num1, opr, num2):
#This is a simple utility function skipping type check
if opr=="+":
return num1+num2
elif opr=="-":
return num1-num2
elif opr=="*":
return num1*num2
elif opr=="/":
return num1/num2
elif opr=="^":
return num1**num2
else:
return None

def calc(expr):
#expr: nonempty string that is an arithmetic expression
#the fuction returns the calculated result
expr = cleanUp(expr)
if len(expr)<=0:
print("argument error: line A in eval_expr")        #Line A
return "argument error: line A in eval_expr"
#Hold two modes: "addition" and "multiplication"
#Initializtion: get the first number
newNumber, newOpr, oprPos = getNextNumber(expr, 0)
ops = ["^","*","/","+","-"]
if newNumber is None or not isNumber(newNumber):
print("input formula error: line B in eval_expr")   #Line B
return "input formula error: line B in eval_expr"
elif newOpr is None or not newOpr in ops:
return newNumber
cacheNum=newNumber
pos=oprPos+1                #the new current position
opr=newOpr                  #the new current operator
#start the calculation. Use the above functions effectively.
new_expr = expr
while True:
newNumber, newOpr, oprPos = getNextNumber(new_expr, pos)
#print(cacheNum, opr, newNumber, "The next opr however is", newOpr)
if (not isNumber(newNumber)):
print("input formula error: line C in eval_expr")   #Line C
return "input formula error: line C in eval_expr"
if (newOpr == None):
break
if(newOpr == "^"):
temp = getNextNumber(new_expr, oprPos+1)[0]
if (not isNumber(temp)):
print("input formula error: line D in eval_expr")   #Line D
return "input formula error: line D in eval_expr"
#print("Weee", newOpr, newNumber, "with", temp)
subs = exeOpr(float(newNumber), newOpr, float(temp))
del new_expr[oprPos-1]
del new_expr[oprPos-1]
new_expr[oprPos-1] = str(subs)
#print("The new expression to start calculating is: " + new_expr)
elif((newOpr == "*" or newOpr == "/") and (opr not in ["^","*","/"])):
temp, temp_Opr, temp_Pos = getNextNumber(new_expr, oprPos+1)
#print("uhhh", temp_Opr)
if temp_Opr == "^":
temp2 = getNextNumber(new_expr, oprPos+3)[0]
if (not isNumber(temp)):
print("input formula error: line D in eval_expr")   #Line D
return "input formula error: line D in eval_expr"
#print("We", temp_Opr, temp, "with", temp2)
subs = exeOpr(float(temp), temp_Opr, float(temp2))
del new_expr[temp_Pos-1]
del new_expr[temp_Pos-1]
new_expr[temp_Pos-1] = str(subs)
#print("The new expression to start calculating is: " + new_expr)
else:
if (not isNumber(temp)):
print("input formula error: line D in eval_expr")   #Line D
return "input formula error: line D in eval_expr"
#print("Weee", newOpr, newNumber, "with", temp)
subs = exeOpr(float(newNumber), newOpr, float(temp))
del new_expr[oprPos-1]
del new_expr[oprPos-1]
new_expr[oprPos-1] = str(subs)
#print("The new expression to start calculating is: " + new_expr)
elif((newOpr == "+" or newOpr == "-") and opr not in ["+","-"]):
subs = exeOpr(float(cacheNum), opr, float(newNumber))
del new_expr[pos-2]
del new_expr[pos-2]
new_expr[pos-2] = str(subs)
cacheNum=str(subs)
opr=newOpr
#print("The new expressionn to start calculating is: " + new_expr)
else:
subs = exeOpr(float(cacheNum), opr, float(newNumber))
del new_expr[pos-2]
del new_expr[pos-2]
new_expr[pos-2] = str(subs)
#print("The new expression to start calculating is: " + new_expr)
cacheNum=str(subs)
opr=newOpr
subs = exeOpr(float(cacheNum), opr, float(newNumber))
#print(new_expr)
del new_expr[new_expr.index(cacheNum)]
del new_expr[new_expr.index(opr)]
new_expr[new_expr.index(newNumber)] = str(subs)
return float(new_expr[0])

print(calc("2+2 -4 * 1 / 3.344 - -2")


## closed as off-topic by Vogel612♦, Dannnno, Donald.McLean, ChrisWue, GraipherSep 28 '17 at 14:13

This question appears to be off-topic. The users who voted to close gave this specific reason:

• "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Vogel612, Dannnno, Donald.McLean, ChrisWue, Graipher
If this question can be reworded to fit the rules in the help center, please edit the question.

• Your last line is missing a ) at the end. – Richard Neumann Sep 27 '17 at 11:17
• @Ludisposed Fixed it to work, lemme know if you still run into it. – Blanker Sep 27 '17 at 16:39
• The last version returns ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-2'] which is not ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '+', '2'] as advertized by the post. – Mathias Ettinger Sep 28 '17 at 8:51
• @MathiasEttinger Yeah I forgot to change that, but they both evaluate to the same result – Blanker Sep 29 '17 at 1:23

The entire cleanUp method can be changed with almost 1 regular expression

Take the example:

import re
def clean(s):
return re.split("([+\-\/*])", s.replace(" ", ""))

>>> print(clean("2+2 -4 * 1 / 3.344 - -2"))
>>> ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '', '-', '2']


This is almost the same as your output only difference is, it adds an empty '' when there are 2 operaters after eachother.

This can be fixed using a bit of list slicing, making the clean look like this:

import re
def clean(s):
cleaned_expr = re.split("([+\-\/*])", s.replace(" ", ""))
while '' in cleaned_expr:
# Remove empty and concat operator to next number
i = cleaned_expr.index('')
cleaned_expr[i] = cleaned_expr[i+1] + cleaned_expr[i+2]
del cleaned_expr[i+1:i+3]
return cleaned_expr

>>> print(clean("2+2 -4 * 1 / 3.344 - -2"))
>>> ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-2']
>>> print(clean("2+2- -4 * 1 / 3.344 -2"))
>>> ['2', '+', '2', '-', '-4', '*', '1', '/', '3.344', '-', '2']


Some other minor PEP8 improvements are:

1. naming of functions should be clean_up() instead of cleanUp
2. Missing whitespaces around operators like =
3. Missing whitespace around ,

# Note

In retrospect and Thanks to @Mathias your calc() feels buggy, as an honest reviewer I would recommend it handles input like this ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-', '2'] when that is fixed I'm tempted to give another look at it, for now it just feels too buggy.

# The cleanest clean (but incorrect output for your calc)
import re
def clean(s):
s = re.split("([+\-\/*])", s.replace(" ", ""))
return [token for token in s if token]

>>> print(clean("2+2 -4 * 1 / 3.344 - -2"))
>>> ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-', '2']

• That wil not work since, his calc() will break at this input ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-', '2'] – Ludisposed Sep 28 '17 at 9:37
• In the end I think it would be best if it handles ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-', '2'] that output, since it is cleaner then ['2', '+', '2', '-', '4', '*', '1', '/', '3.344', '-', '-2'] – Ludisposed Sep 28 '17 at 9:49
• Thinking back at it, neither version handle the case of 2---2 (should they? Python returns 0) as ['2', '-', '-', '-', '2'] is returned… So yes, better keep clean simple and fix calc instead… – Mathias Ettinger Sep 28 '17 at 9:58
• I think to return 0 is a perfectly valid answer since 2 - + 2 = 0 it feels like I adapt clean() to work for calc() while perhaps it be better to fix calc() first – Ludisposed Sep 28 '17 at 10:04
• Then return [token for token in cleaned_expr if token] should do instead of the while loop ;) – Mathias Ettinger Sep 28 '17 at 10:06

Regardless of the fact, that your code is not running properly as it returns

input formula error: line C in eval_expr
input formula error: line C in eval_expr


with the example input, I'd suggest simplifying your module to

from re import compile

ARITHMETIC_EXPRESSIONS = compile('[0-9]*( ){0,}([+-/*^]( ){0,}[0-9]*( ){0,})*')

def coerce_python_syntax(string):
"""Converts arithmetic expression to python syntax."""

string = string.replace('^', '**')

def calc(string):
"""Calculates the arithmetic expression given by string."""
if ARITHMETIC_EXPRESSIONS.fullmatch(string):
return eval(coerce_python_syntax(string))

raise ValueError('String is not a safe arithmetic expression.')

if __name__ == '__main__':
print(calc("2+2 -4 * 1 / 3.344 - -2"))


since you are implementing basic arithmetic evaluation that python already has down cold.

• Until something like: eval("os.system('rm -rf /')") comes along >;) – Ludisposed Sep 27 '17 at 11:49
• Ahh I see .fullmatch now. I stand corrected, but still I hope you see, why eval is evil – Ludisposed Sep 27 '17 at 13:27
• Without sanitizing the input, definitely. – Richard Neumann Sep 27 '17 at 13:27
• The regex does miss a few, like operater ^ also try calc("01 + 10") or how about a CPU-burner attack? Stuff like 2**9999999999**9999999 – Ludisposed Sep 27 '17 at 13:54
• Point is you'll end up with something quite long if you want to protect yourself for all, safe python – Ludisposed Sep 27 '17 at 13:56