Can this Python postfix notation (reverse polish notation) interpreter be made more efficient and accurate?
Here is a Python postfix notation interpreter which utilizes a stack to evaluate the expressions. Is it possible to make this function more efficient and accurate?
#!/usr/bin/env python
import operator
import doctest
class Stack:
"""A stack is a collection, meaning that it is a data structure that
contains multiple elements.
"""
def __init__(self):
"""Initialize a new empty stack."""
self.items = []
def push(self, item):
"""Add a new item to the stack."""
self.items.append(item)
def pop(self):
"""Remove and return an item from the stack. The item
that is returned is always the last one that was added.
"""
return self.items.pop()
def is_empty(self):
"""Check whether the stack is empty."""
return (self.items == [])
# Map supported arithmetic operators to their functions
ARITHMETIC_OPERATORS = {"+":"add", "-":"sub", "*":"mul", "/":"div",
"%":"mod", "**":"pow", "//":"floordiv"}
def postfix(expression, stack=Stack(), operators=ARITHMETIC_OPERATORS):
"""Postfix is a mathematical notation wherein every operator follows all
of its operands. This function accepts a string as a postfix mathematical
notation and evaluates the expressions.
1. Starting at the beginning of the expression, get one term
(operator or operand) at a time.
* If the term is an operand, push it on the stack.
* If the term is an operator, pop two operands off the stack,
perform the operation on them, and push the result back on
the stack.
2. When you get to the end of the expression, there should be exactly
one operand left on the stack. That operand is the result.
See http://en.wikipedia.org/wiki/Reverse_Polish_notation
>>> expression = "1 2 +"
>&g开发者_运维知识库t;> postfix(expression)
3
>>> expression = "5 4 3 + *"
>>> postfix(expression)
35
>>> expression = "3 4 5 * -"
>>> postfix(expression)
-17
>>> expression = "5 1 2 + 4 * + 3 -"
>>> postfix(expression, Stack(), ARITHMETIC_OPERATORS)
14
"""
if not isinstance(expression, str):
return
for val in expression.split(" "):
if operators.has_key(val):
method = getattr(operator, operators.get(val))
# The user has not input sufficient values in the expression
if len(stack.items) < 2:
return
first_out_one = stack.pop()
first_out_two = stack.pop()
operand = method(first_out_two, first_out_one)
stack.push(operand)
else:
# Type check and force int
try:
operand = int(val)
stack.push(operand)
except ValueError:
continue
return stack.pop()
if __name__ == '__main__':
doctest.testmod()
General suggestions:
- Avoid unnecessary type checks, and rely on default exception behavior.
has_key()
has long been deprecated in favor of thein
operator: use that instead.- Profile your program, before attempting any performance optimization. For a zero-effort profiling run of any given code, just run:
python -m cProfile -s cumulative foo.py
Specific points:
list
makes a good stack out of the box. In particular, it allows you to use slice notation (tutorial) to replace thepop
/pop
/append
dance with a single step.ARITHMETIC_OPERATORS
can refer to operator implementations directly, without thegetattr
indirection.
Putting all this together:
ARITHMETIC_OPERATORS = {
'+': operator.add, '-': operator.sub,
'*': operator.mul, '/': operator.div, '%': operator.mod,
'**': operator.pow, '//': operator.floordiv,
}
def postfix(expression, operators=ARITHMETIC_OPERATORS):
stack = []
for val in expression.split():
if val in operators:
f = operators[val]
stack[-2:] = [f(*stack[-2:])]
else:
stack.append(int(val))
return stack.pop()
Lists can be used directly as stacks:
>>> stack = []
>>> stack.append(3) # push
>>> stack.append(2)
>>> stack
[3, 2]
>>> stack.pop() # pop
2
>>> stack
[3]
You can also put the operator functions directly into your ARITHMETIC_OPERATORS dict:
ARITHMETIC_OPERATORS = {"+":operator.add,
"-":operator.sub,
"*":operator.mul,
"/":operator.div,
"%":operator.mod,
"**":operator.pow,
"//":operator.floordiv}
then
if operators.has_key(val):
method = operators[val]
The goal of these is not to make things more efficient (though it may have that effect) but to make them more obvious to the reader. Get rid of unnecessary levels of indirection and wrappers. That will tend to make your code less obfuscated. It will also provide (trivial) improvements in performance, but don't believe that unless you measure it.
Incidentally, using lists as stacks is fairly common idiomatic Python.
You can directly map the operators: {"+": operator.add, "-": operator.sub, ...}
. This is simpler, doesn't need the unnecessary getattr
and also allows adding additional functions (without hacking the operator module).
You could also drop a few temporary variables that are only used once anyway:
rhs, lhs = stack.pop(), stack.pop()
stack.push(operators[val](lhs, rhs)).
Also (less of a performance and more of a style issue, also subjective), I would propably don't do error handling at all in the loop and wrap it in one try block with an except KeyError
block ("Unknown operand"), an except IndexError
block (empty stack), ...
But accurate? Does it give wrong results?
精彩评论