5
\$\begingroup\$

This is my first ever project I've coded in Python so I'm sorry if it's a little novice. I'm just looking for beginner feedback on what I could improve next time.

There is one issue with this calculator: let's say I add 1 and 2, I expect to get 3, instead it returns 3.0, but I'm guessing that's because I'm casting to a float.

The reason I'm casting to a float is to accept decimals in the equation. It would be nicer if it displayed the whole number without the .0

while True:
    calculation = input("Calculation: ")

    if (calculation.__contains__("+")):
        print(float(calculation.split('+')[0]) + float(calculation.split('+')[1]))

    elif (calculation.__contains__("-")):
        print(float(calculation.split('-')[0]) - float(calculation.split('-')[1]))

    elif (calculation.__contains__("*")):
        print(float(calculation.split('*')[0]) * float(calculation.split('*')[1]))

    elif (calculation.__contains__("/")):
        print(float(calculation.split('/')[0]) / float(calculation.split('/')[1]))
\$\endgroup\$
5
\$\begingroup\$

Pythons magic methods are there to make your life easier, not harder. They allow custom classes to use built-in functionality. They are not there so you have to use long names with built-in types.

The __contains__ method is called under the hood whenever you use the in keyword.

In addition, you do the same thing for all four operations: See if it is present, split by the operator and apply it. This can be simplified using a loop over a dictionary, mapping the string of the operator to the operator itself (using the operator module which gives you functions for most operations).

And so your code can be simplified to:

import operator

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

while True:
    calculation = input("Calculation: ")
    for name, op in OPERATIONS.items():
        if name in calculation:
            a, b = map(float, calculation.split(name))
            print(op(a, b))

And at that point you should ask yourself how to modify this to allow multiple operations or even grouping expressions with parenthesis.

You should also probably add some error handling (what happens if the user tries to enter multiple operations? What happens if they do not enter a number?).

\$\endgroup\$
2
\$\begingroup\$

What you are doing is parsing your input string the use the various elements to make the sum. For this you can use Regular Expressions by importing the Re module. See below example how you can do this in your example. Note it can also handle negative numbers at the start of the input.

import re

while True:
    calculation = input("Calculation: ")

    parse = re.match(r'([+-]*[\d.]*?)(?=[^\d^.])([\s+-/*]*?)(?=[\d])([\d.]*)$',
                     calculation)

    num1 = float(parse.group(1).strip())
    operand = parse.group(2).strip()
    num2 = float(parse.group(3).strip())

    if operand == '*':
        answer = num1 * num2

    elif operand == '/':
        answer = num1 / num2

    elif operand == '+':
        answer = num1 + num2

    elif operand == '-':
        answer = num1 - num2

    if int(answer) == answer:
        answer = int(answer)

    print(f'{calculation} = {answer}')

There is no check if something is entered that cannot be parsed.

\$\endgroup\$
  • \$\begingroup\$ Why does the regex allow \s to be part of group 2? \$\endgroup\$ – 200_success Jul 21 '18 at 16:28
  • \$\begingroup\$ the \s allows for spaces around the operand. See for example docs.python.org/3/library/re.html. \$\endgroup\$ – Bruno Vermeulen Jul 21 '18 at 23:30
  • \$\begingroup\$ A better regex would do the job without needing .strip(), and would also disallow invalid expressions such as 2 3. Also note that the character class for the operator is wrong, since the minus creates a range. I suggest r'([+-]?\d*(?:\.\d+)?)\s*([+*/-])\s*([+-]\d*(?:\.\d+)?)'. \$\endgroup\$ – 200_success Jul 22 '18 at 1:20
  • \$\begingroup\$ Thanks yes this is a much better expression. \$\endgroup\$ – Bruno Vermeulen Jul 22 '18 at 3:15

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.