10
\$\begingroup\$

I'm very new to Python, and so I'm trying to learn it by doing simple programs, like a calculator. But I don't know if this is the "Pythonic" way to do it, or if I am making beginner errors.

So could someone please tell if something is wrong with it, and how to improve it?

Here is some sample input and output:

Enter the first number: 2
Enter the second number: 10
Enter an operator (valid operators are +, -, *, / and pow): pow
1024.0

Here is the code

#A basic calculator which supports +, -, *, / and pow

def add(a, b):
    return a + b

def sub(a, b):
    return a - b

def mul(a, b):
    return a * b

def div(a, b):
    return a / b

def pow(a, b):
    return a ** b

if __name__ == "__main__":
    while True:
        try:
            number1 = float(input("Enter the first number: "))
            number2 = float(input("Enter the second number: "))
        except:
            print("That is not a number!")
            continue

        operator = input("Enter an operator (valid operators are +, -, *, / and pow): ")

        #Check if user operator is a valid one
        if operator == "+":
            func = add
        elif operator == "-":
            func = sub
        elif operator == "*":
            func = mul
        elif operator == "/":
            func = div
        elif operator == "pow":
            func = pow
        else:
            print("Invalid operator!")
            continue

        #Print result
        print(func(number1, number2))
\$\endgroup\$
  • \$\begingroup\$ How do you quit the program? \$\endgroup\$ – Mathias Ettinger May 1 '16 at 11:05
  • 1
    \$\begingroup\$ @MathiasEttinger I just kill the process, not very user friendly but it works :) \$\endgroup\$ – Rakete1111 May 1 '16 at 11:08
13
\$\begingroup\$

For a beginner this is pretty good. But, there are three glaring problems with it.

  1. You should use a 'main' function.
  2. You should use a dictionary.
  3. operators

Starting with (2), a dictionary holds a key value pair. If you've come across lists/arrays, it's like them, apart from that you can use a lot of different types, not just integers.

Defining one is pretty easy, and using it will reduce your amount of code.

operators = {
    "+": add,
    "-": sub,
    "*": mul,
    "/": div,
    "pow": pow
}

This will define an object to hold all the operators that you can use. Now all we need to do is use it. Using operators.get it's easy.

func = operators.get(operator, None)
if func is None:
    print("Invalid operator!")
    continue

Lovely and simple.

The main function is simple, just move everything in if __name__ == '__main__': into a function. With the operator addition you shouldn't really need to make smaller functions, but it may be a good idea, making a function to get user input, could be a good idea. But it's up to you. If you did want to do both of these, you'd do something like:

def main():
    while True:
        number1, number2, operator = get_user_input()
        func = operators.get(operator, None)
        if func is None:
            print("Invalid operator!")
            continue

        print(func(number1, number2))

if __name__ == '__main__':
    main()

Just to note, you may not want to make get_user_input.

Something that you may not know is you can use the builtin operators library. This will allow you to remove all your definitions of add, sub, etc. And will allow you to change operators to:

import operator

operators = {
    "+": operator.add,
    "-": operator.sub,
    "*": operator.mul,
    "/": operator.truediv,
    "pow": operator.pow
}

There's some history about Python's division, and that's why it's called truediv, but if you haven't and won't use Python2, then you don't really need to know.

\$\endgroup\$
  • \$\begingroup\$ Thanks! I didn't know about operators \$\endgroup\$ – Rakete1111 May 1 '16 at 11:09
  • \$\begingroup\$ should I only call the main function if __name__ is equal to __main__, or should I just call it, ignoring it? \$\endgroup\$ – Rakete1111 May 1 '16 at 11:13
  • 2
    \$\begingroup\$ @BlitzRakete: Only if it is __main__. If you were to do it whatever __name__ equaled, the if would be pointless. As it is, it is checking if the file is being run as a program vs. being imported. \$\endgroup\$ – zondo May 1 '16 at 11:14
  • 2
    \$\begingroup\$ @BlitzRakete: It's a pleasure. For more information, see What does if __name__ == “__main__” do? \$\endgroup\$ – zondo May 1 '16 at 11:19
6
\$\begingroup\$

1

Despite the fact that bare except works, it would be nicer to be more explicit about what exception you are catching. I would write

except ValueError:

2

pow shadows away the function provided by Python; I would rename it to, say, mypow. Not that it matters in this case, but in a larger program you should not do this: finding bugs due to a shadowed identifier is hard.

3

You can rewrite the actual logic much more succintly by using a dictionary mapping the name of an operator to the actual function performing the operation.

Summa summarum

All in all, I had this in mind:

    def add(a, b):
        return a + b


    def sub(a, b):
        return a - b


    def mul(a, b):
        return a * b


    def div(a, b):
        return a / b


    def mypow(a, b):
        return a ** b


    if __name__ == "__main__":
        operator_map = {"+": add, "-": sub, "*": mul, "/": div, "pow": mypow}

        while True:
            try:
                number1 = float(input("Enter the first number: "))
                number2 = float(input("Enter the second number: "))
            except ValueError:
                print("That is not a number!")
                continue

            operator = input("Enter an operator (valid operators are +, -, *, / and pow): ")
            print(operator_map[operator](number1, number2) if operator in operator_map else "Invalid operator!")

Hope that helps.

\$\endgroup\$
5
\$\begingroup\$

Don't ever use input in a function that does work. It violates "separation of concerns" by tying the abstract calculation logic to the means by which the input is obtained.

Far better would be:

def binary_operation(operation, left, right):
    """Returns the result of applying operation to its arguments."""
    return operator_map[operation](left, right)

where operator_map is taken from @coderodde's answer. This allows you to call your (simple) engine in ways that have no connection to how the operands get there. For example:

binary_operation('+', 2, 3)
binary_operation(sys.argv[1], float(sys.argv[2]), float(sys.argv[3]))

you can also have the original behavior with

binary_operation(input('operator'), float(input('number')), float(input('number'))

or even

binary_operation('*', 5.0 binary_operation('+' 2, 3))

Prior to posting, I find that Joe Wallis suggested this, but I think it important enough to elaborate on its own.

\$\endgroup\$
3
\$\begingroup\$

You don't need to define all of those functions because the operator module already did, and they happen to have the same names as yours.

You use __name__. That's great. Since you won't be defining functions anymore (because of ^^), this file will be completely useless as a module. Therefore, I probably wouldn't bother with it. I might define a function to get input and then have a while True:

def get_answer(num1, num2, op):
    ...
if __name__ == '__main__':
    while True:
        try:
            number1 = float(input(...))
            number2 = float(input(...))
        except ValueError:
            print("That is not a number!")
            continue
        operator = input(...)
        answer = get_answer(number1, number2, operator)
        if answer is None:
            print("Invalid operator!")
        else:
            print(answer)

That's just a rough draft. I probably wouldn't do that, but it is a possibility.

Never have a bare except unless you are writing your own interpreter. You should have expectations for what errors you might get. Let's say you mis-spelled number1 and said numberl. There would be an error, so it would say That is not a number! ... no matter what you type. What is your expectation? Of course, it's that the user might type an invalid float. In that case, use except ValueError:

I would use a dictionary for the different operations, and I would use the more standard operator names. That is, ^ instead of pow. I would also tell the user the options at the beginning instead of telling him each time. Here is the full program:

import operator

operators = {
    '+': operator.add,
    '-': operator.sub,
    '*': operator.mul,
    '/': operator.truediv,
    '%': operator.mod,
    '^': operator.pow,
    '//': operator.floordiv,
    '>': operator.gt,
    '<': operator.lt,
    '=': operator.eq,
    '!=': operator.ne,
    '>=': operator.ge,
    '<=': operator.le,
}

def get_answer(num1, num2, op):
    try:
        return operators[op](num1, num2)
    except IndexError:
        raise ValueError("Invalid operator")

if __name__ == '__main__':
    operator_string = ', '.join(operators)
    print("Valid operators: {}\n".format(operator_string))
    while True:
        try:
            number1 = float(input("Enter the first number: "))
            number2 = float(input("Enter the second number: "))
        except ValueError:
            print("That is not a number!")
            continue
        except (KeyboardInterrupt, EOFError):
            break # exit
        op = input("Enter an operator: ")
        try:
            print(get_answer(number1, number2, op))
        except ValueError:
            print("Invalid operator!")
\$\endgroup\$

protected by Jamal Feb 20 at 4:16

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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