Why is my postfix evalutation function not working?

Question

This is what I have so far, this is a continutation of some pythoncode my classmate wrote and we keep getting lots of errors.

I think the errors lie somewhere in the if and for loop but I am fairly new to coding so any help would be appriciated.

def postfix(expr_str: str) -> float:
    op = ['+', '-', '*', '/']
    mystack = []
    ch = expr_str.split()
    
    if ch not in op:
        mystack.append(ch)
    for ch in expr_str:
        if ch.isdigit():
            mystack.append(ch)
        else:
            op_2 = mystack.pop() # remove mst recent operand from stack first
            op_1 = mystack.pop() # remove first operand from the stack after
            if ch == '+':
                mystack.append(op_1 + op_2)
            elif ch == '-':
                mystack.append(op_1 - op_2)
            elif ch == '/':
                mystack.append(op_1 - op_2)
            elif ch == '*':
                mystack.append(op_1 / op_2)
            mystack.append(mystack)
    return (''.join(mystack))

I already tried to run this through coding rooms but there were many errors.

Answer 1

There are quite a few problems in your code:

• if ch not in op makes no sense, because ch is at that moment a list of strings -- which you got from expr_str.split(). This if block should not be there, nor the assignment to ch. And by consequence the variable op is also not used anymore. Just remove that code.

• for ch in expr_str: will read individual characters from the input string. This would split "123" into "1", "2", and "3". Instead, here it would be a good moment to do for ch in expr_str.split(), assuming that the given expression is space separated, like for instance "1 2 3 * +".

• mystack.append(op_1 + op_2) assumes that op_1 and op_2 are numeric, but your code only puts strings on the stack, not numbers. So change mystack.append(ch) (the one after the isdigit test) to mystack.append(int(ch))

• You haven't applied multiplication and division where you're supposed to do that.

• mystack.append(mystack) makes no sense: it would duplicate the contents of the stack. This statement should be removed.

• After the above corrections, ''.join(mystack) will not work, because now the stack has numbers, not strings. In fact, if the input expression was semantically correct, there should be exactly one number on the stack, so you should actually do return stack[0] or return stack[-1]

With the above fixes your code would look like this:

def postfix(expr_str) -> float:
    mystack = []
    for ch in expr_str.split():
        if ch.isdigit():
            mystack.append(int(ch))
        else:
            op_2 = mystack.pop()
            op_1 = mystack.pop()
            if ch == '+':
                mystack.append(op_1 + op_2)
            elif ch == '-':
                mystack.append(op_1 - op_2)
            elif ch == '/':
                mystack.append(op_1 / op_2)
            elif ch == '*':
                mystack.append(op_1 * op_2)
    return (mystack[0])

You could further improve by checking for errors in the input string, like an invalid operator (add an else after the last elif), or too few operands (that happens when the list is empty when you are about to pop), or too many operands (this happens when at the end the stack has more than one number), or the input is empty,... etc.

Example run:

print(postfix("1 2 3 * +"))  # 7