8
\$\begingroup\$

Can I code this Simple Calculator in a more efficient way?

I'm currently self-taught, one thing I notice is that I get an error if I don't press the space bar after each entry, is there a way to avoid that error?

x,y,z = input(" Enter Your Math Problem ").split()

x = int(x)
y = str(y)
z = int(z)

if y == "+":
    answer = x + z
    print(str(x) + y + str(z) + " = " + str(answer) )
elif y == "-":
    answer = x - z
    print(str(x) + y + str(z) + " = " + str(answer) )
elif y == "/":
    answer = x / z
    print(str(x) + y + str(z) + " = " + str(answer) )
elif y == "*":
    answer = x * z
    print(str(x) + y + str(z) + " = " + str(answer) )
else:
    print("Invalid Operator")
\$\endgroup\$

1 Answer 1

15
\$\begingroup\$

Variable Names

Your variable names need some improvement. When I see x, y, and z, I immediately expect those variables to be the same kind of thing: numbers. Your y is a string, and represents the “operator” for the expression. Let’s fix that:

x, op, y = input(" Enter Your Math Problem ").split()

Now I see x and y — two numbers — and an op, or operation, to perform. Much clearer.

Useless code

The operator is already a string, so y = str(y) (when y was the operator) is a do nothing operation. We can just get rid of it.

x = int(x)
y = int(y)

Print formatting

Your print statement print(str(x) + y + str(z) + " = " + str(answer) ) has a lot of “noise”. You’ve got 3 str() calls in it to turn numbers back into strings, and 4 concatenation operators (+) to join individual strings together.

Python has many different format functions, but perhaps the clearest is using f-strings, available in Python 3.7 and later. You could replace 4 of your print statements with 4 copies of the following statement. Note the f in front of the quotation-mark; that flags the string as a “formatted-string”. Expressions inside {…} are evaluated and interpolated into the string. No str(…) calls, and no + operators necessary.

    print(f"{x} {op} {y} = {answer}")

DRY: Don’t Repeat Yourself

You want your code to be DRY; WET (Write Everything Twice) is to be avoided. So we don’t actually want 4 copies of exactly the same print statement. We only want one.

We could move the print statement after the if…elif…elif…elif…else statement, but then it would execute after each of the 5 different cases, where as we only want it to execute for 4 of them. There are a few different ways to accomplish this, but without introducing too many new concepts, let’s just use a nested if structure:

if op in {"+", "-", "/", "*"}:

    if op == "+":
        answer = x + z
    elif op == "-":
        answer = x - z
    elif op == "/":
        answer = x / z
    else:
        answer = x * z

    print(f"{x} {op} {y} = {answer}")

else:
    print("Invalid Operator")

The in operator checks if op is one of the elements of the set of 4 supported operators. If it is, we compute answer based on the exact value of op, and then print out the solved expression at the end. If op isn’t a supported operator, we print “Invalid Operator”.

Single Source of Truth

You never want to have to maintain something in more than one place. Unfortunately, the above nested if statement does just that. If we wanted to add an exponentiation operator (^ or **), we’d have to add it to the if op in {…}: set as well as to the nested if…elif…elif…else block.

Instead, we can do something very different:

if op == "+":
    answer = x + z
elif op == "-":
    answer = x - z
elif op == "/":
    answer = x / z
elif op == "*":
    answer = x * z
else:
    raise NotImplementedError(f"Invalid Operator: {op}")

print(f"{x} {op} {y} = {answer}")

Unlike the previous code, we explicitly test elif op == "*":, which is more consistent. If the op doesn’t match any of the defined operators, we execute the else: statement, which raises an Exception, which left uncaught will terminate the program without executing the following print statement.

Dictionary of functions

Functions in Python are first-class objects, which means they can be held in variables, and more importantly, dictionaries. All the built-in operators are available as functions in the operator module:

import operator

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

x, op, y = input(" Enter Your Math Problem ").split()

x = int(x)
y = int(y)

if op in OPERATIONS:
    operation = OPERATIONS[op]
    answer = operation(x, y)
    print(f"{x} {op} {y} = {answer}")
else:
    print(f"Invalid Operator: {op}")

Splitting Input

To solve your issue of the input not splitting if there is no space between the numbers and the operator, one usually resorts to regular expressions. However, they are not exactly simple.

To handle 123+456, one might use the regular expression (\d+)\s*([-+/*])\s*(\d+).

That regular expression doesn’t work for -123 * -456, which your current code can easily handle. For that, you’d need something a little more complex to handle a leading negative flag. Perhaps: (-?\d+)\s*([-+/*])\s*(-?\d+).

But that would fail on the perfectly valid -123 * +456, again which your original program handled without issue.

I think ([-+]?\d+)\s*([-+/*])\s*([-+]?\d+) does the trick, but it looks like gobblygook.

Without explanation (or testing):

import operator, re

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

problem = input(" Enter Your Math Problem ")

if m := re.fullmatch(r"([-+]?\d+)\s*([-+/*])\s*([-+]?\d+)", problem):
    x = int(m[1])
    op = m[2]
    y = int(m[3])

    answer = OPERATIONS[op](x, y)
    print(f"{x} {op} {y} = {answer}")
else:
    print(f"Invalid or unsupported math problem: {problem}")

Note that we are back to our multiple sources of truth. We have OPERATIONS which contains all known operators, and the regular expression which also contains the known operators. It would be possible to build up the regular expression from the keys of the OPERATIONS dictionary, but that involves even more complexity.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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