2
\$\begingroup\$

This is my solution for evaluating postfix expression using a stack. It does work on multidigit numbers, the problem I was struggling with last time. Now it works completely fine.

This is the stack that I'm using:

class Stack:
    def __init__(self):
        self.items = []
    
    def isEmpty(self):
        return self.items == []
    
    def push(self, item):
        self.items.append(item)
    
    def pop(self):
        self.items.pop()
    
    def peek(self):
        try:
            return self.items[-1]
        except:
            return False
    
    def value_at_index(self, value):
        try:
            return self.items[value]
        except:
            return False
    
    def all_items(self):
        return self.items

And here is my code:

def result(data):
    """Evaluating postfix expression"""
    stack = Stack()
    if data != False:
        for element in data:
            try:
                if type(float(element)) == float:
                    stack.push(element)
            except:
                method = "float(stack.value_at_index(-2))" + element + "float(stack.value_at_index(-1))"
                method = eval(method)
                stack.pop()
                stack.pop()
                stack.push(method)
        return stack.peek()
    else:
        return "Check your formula"

data = ['20', '10', '+', '75', '45', '-', '*']
print(result(data))

Output:

900.00

It works perfectly fine, but to be completely honest i would like to get rid of eval because of the negative opinions on this built-in function I heard. I want some reviews from you guys and advices how to generally improve it.

Using Python 3.6.7.

\$\endgroup\$
3
  • \$\begingroup\$ isn't index_of_element actually value_at_index? \$\endgroup\$
    – hjpotter92
    Nov 14 at 13:50
  • \$\begingroup\$ @hjpotter92 well probably yeah, my bad at naming. \$\endgroup\$ Nov 14 at 13:54
  • \$\begingroup\$ I'm curious: For what value of element have you observed type(float(element)) == float to be false? \$\endgroup\$ Nov 14 at 15:17
1
\$\begingroup\$
  • Python's lists are stacks.
    Your Stack class is making your code harder to read because it's not using Python's idioms.

  • Don't use bare excepts, as all exceptions are silenced.
    You shouldn't be silencing SystemExit or KeyboardInterrupt. Only handle the exceptions you need to handle.

  • type(float(element)) == float
    You should default to using isinstance. You'll be building a habit of supporting child types, rather than fighting bad habits later.

  • I'd suggest splitting normalizing your tokens away from evaluating the tokens. Basically move float(element) out of the function.

  • We can then change your type check to check if the number is a numbers.Number.

  • I'd recommend preferring storing numbers as ints rather than floats to avoid floating point issues.

import numbers
from typing import Union, Iterable, Iterator

AToken = Union[str, numbers.Number]


def to_numbers(tokens: Iterable[str]) -> Iterator[AToken]:
    for token in tokens:
        for cast in (int, float, complex):
            try:
                yield cast(token)
                break
            except ValueError:
                pass
        else:
            yield token


def evaluate(data: Iterable[AToken]) -> numbers.Number:
    stack: list[numbers.Number] = []
    for token in data:
        if isinstance(token, numbers.Number):
            stack.append(token)
        else:
            rhs = stack.pop()
            lhs = stack.pop()
            stack.append(eval(f"{lhs}{token}{rhs}"))
    return stack[0]


data = ['20', '10', '+', '75', '45', '-', '*']
print(evaluate(to_numbers(data)))

We can remove the need for using eval by calling the appropriate dunder method.

>>> getattr(1, "__add__")(2)
3

So we can just pass in a dictionary of {op: dunder}.

def evaluate(data: Iterable[AToken], ops: dict[str, str]) -> numbers.Number:
    stack: list[numbers.Number] = []
    for token in data:
        if isinstance(token, numbers.Number):
            stack.append(token)
        else:
            rhs = stack.pop()
            lhs = stack.pop()
            stack.append(getattr(lhs, ops[token])(rhs))
    return stack[0]


OPS = {
    "+": "__add__",
    "-": "__sub__",
    "*": "__mul__",
}

data = ['20', '10', '+', '75', '45', '-', '*']
print(evaluate(to_numbers(data), OPS))
\$\endgroup\$
5
  • \$\begingroup\$ Now if im trying use the second part of your code without eval, im getting TypeError: 'type' object is not subscriptable \$\endgroup\$ Nov 14 at 15:04
  • 1
    \$\begingroup\$ @KermitTheFrog You're probably not on Python 3.9+. Put from __future__ import annotations at the top of the file and the issues will disappear. Alternately change dict[str, str] to "dict[str, str]" and list[numbers.Number] to "list[numbers.Number]". \$\endgroup\$
    – Peilonrayz
    Nov 14 at 15:14
  • \$\begingroup\$ Gotcha, i have one more question, how would it be possible to have input f.e not as splitted strings but a one input: ['20 10 + 75 45 - *'] \$\endgroup\$ Nov 14 at 22:09
  • \$\begingroup\$ i meant ('20 10 + 75 45 - *') \$\endgroup\$ Nov 15 at 14:14
  • \$\begingroup\$ @KermitTheFrog you've probably used a poor example because you can just '20 10 + 75 45 - *'.split(). There are two ways you can parse the string otherwise; build a tokenizer or use *BNF (EBNF and ABNF are far nicer than BNF). You can build a parser quite easily by 1. if char in "0123456789": token.append(char) else: yield "".join(token); if char in "+-*": yield char. \$\endgroup\$
    – Peilonrayz
    Nov 15 at 22:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.