10
\$\begingroup\$

I made a prefix notation calculator.

Prefix notation, aka Polish notation, is a form of notation for logic, arithmetic and algebra, like infix and postfix (reverse Polish) notation.

The expression '+ 3 4' in prefix notation is as valid as '3 + 4' in infix notation, as well as '3 4 +' in postfix notation.

The calculator asks for one value per step, and asks about everything step by step.

"""
Pfxcalc
Programmed by zyabin101
This code is hereby released into the public domain
"""

import ast

def lit_input(prompt):
    """
    Safely evaluate input. Only returns the following Python literal
    structures: strings, numbers, tuples, lists, dicts, booleans, and 'None'.

    Raises ValueError for malformed strings.
    """
    return ast.literal_eval(raw_input(prompt))

def lorr_input(prompt, result):
    inputted = raw_input(prompt)
    return result if inputted == "result" else ast.literal_eval(inputted)

# Initialization
x = None

# Print welcome message
print "Welcome to Pfxcalc. Type 'help' for more information."

# Main program loop
while True:
    # Get user input
    cmd = raw_input(">> ")
    # Process input
    if cmd == "quit":
        break
    elif cmd == "result":
        print x
    elif cmd == "help":
        helpmsg = """
        Pfxcalc is a prefix notation calculator.
        Prefix notation, aka Polish notation, is a form of notation for logic,
        arithmetic and algebra, like infix and postfix (reverse Polish)
        notation.

        The expression '+ 3 4' in prefix notation is as valid as '3 + 4' in
        infix notation, as well as '3 4 +' in postfix notation.

        Every computation starts with entering a command and the result is
        printed *and* saved in a variable that can be recalled by entering
        'result'. The variable can even be used as an argument for functions,
        using the same command.

        List of commands:
        quit - self explanatory
        result - recall the result variable
        help - print this message
        + - add two numbers or sequences
        - - subtract two numbers
        * - multiply two numbers, or a sequence by a number
        / - divide two numbers
        """
        print helpmsg
    elif cmd == "+":
        x = (lorr_input("Left side addend? ", x) +
             lorr_input("Right side addend? ", x))
        print x # Implicitly output result
    elif cmd == "-":
        x = lorr_input("Minuend? ", x) - lorr_input("Subtrahend? ", x)
        print x # Implicitly output result
    elif cmd == "*":
        x = (lorr_input("Left side factor? ", x) *
             lorr_input("Right side factor? ", x))
        print x # Implicitly output result
    elif cmd == "/":
        x = lorr_input("Dividend? ", x) / lorr_input("Divisor? ", x)
        print x # Implicitly output result
    else:
        print "Unrecognized command."

I'm interested in code style and problems the code has. What can be fixed here?

\$\endgroup\$
  • 1
    \$\begingroup\$ Why not just print """....""" instead of print helpmsg? It's less assignment, and probably more efficient. \$\endgroup\$ – Conor O'Brien Jun 18 '16 at 19:15
  • \$\begingroup\$ Most if not all single line comments seem superfluous. \$\endgroup\$ – Fatalize Jun 18 '16 at 19:16
  • \$\begingroup\$ I assume this is python2.7, because no print parantheses? \$\endgroup\$ – Riker Jun 18 '16 at 19:16
  • \$\begingroup\$ @EasterlyIrk Correct. \$\endgroup\$ – zyabin101 Jun 18 '16 at 19:26
  • \$\begingroup\$ The lit_input() function is probably unnecessary, because it is defined but never used. \$\endgroup\$ – zyabin101 Jun 18 '16 at 19:28
8
\$\begingroup\$

Use a shebang at the beginning

This is something like #!/usr/bin/env python2, and helps interpreters know how to run it.

Remove some of the unnecessary comments

Comments are good. Excessive comments are just hard to read.

e.g.

elif cmd == "+":
    x = (lorr_input("Left side addend? ", x) +
         lorr_input("Right side addend? ", x))
    print x # Implicitly output result

Turn that into:

elif cmd == "+":
    x = (lorr_input("Left side addend? ", x) +
         lorr_input("Right side addend? ", x))
    print x

And:

# Get user input
cmd = raw_input(">> ")
# Process input
if cmd == "quit":
    ....

Into:

cmd = raw_input(">> ")
# Process user input
if cmd == "quit":
    ....

Empty lines for readability

Using the previous example:

cmd = raw_input(">> ")
# Process user input
if cmd == "quit":
    ....

Into:

cmd = raw_input(">> ")

# Process user input
if cmd == "quit":
    ....

if __name__ == "__main__" check

This just checks if you are running this directly, or from a module import/something else.

Maybe put the while loop into a function called start() and use:

if __name__ == "__main__":
    start()

Don't assign unnecessary variables

The helpmsg var is pretty much useless, just print the plain string.

The lit_input function is not used

Either use it, or remove it. Maybe make a variation of the docstring for the lorr_input?

Other notes:

  • Nice job with the ast safe eval, but a method that doesn't involve eval would also be nice. This is a lot of work, probably unnecessary.

  • I would set x to 0 at the beginning, but that's personal preference.

  • I'm not sure, but you appear to have limited to 87 chars per line (or under). That is a PEP guideline, good job.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Good job! Have your 25 rep, which is 0.225 of my rep (c:). \$\endgroup\$ – zyabin101 Jun 18 '16 at 19:36
  • \$\begingroup\$ @zyabin101 think it's 15, but welcome. You should wait a bit though, give other people a chance to answer. \$\endgroup\$ – Riker Jun 18 '16 at 19:37
  • \$\begingroup\$ 1) It is 25 (10 for upvote, 15 for accept). 2) Got it, thanks for the tip. \$\endgroup\$ – zyabin101 Jun 18 '16 at 19:38
  • \$\begingroup\$ Please do not use #!/usr/bin/env python as a shebang. Default Python versions varies across distributions and environments (e.g. Ubuntu/Debian defaults to Python 2.7, Arch Linux defaults to Python 3.x -- PEP394 only states that distros should point python to 2.x), so try to be as precise as possible. \$\endgroup\$ – ElementW Jun 19 '16 at 10:26
  • \$\begingroup\$ @ElementW okay, will add to the post. \$\endgroup\$ – Riker Jun 19 '16 at 15:59

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.