4
\$\begingroup\$

I have written a program for a simple calculator that can add, subtract, multiply and divide using functions.

Here is my code:

# This function adds two numbers 
def add(x, y):
   return x + y

# This function subtracts two numbers 
def subtract(x, y):
   return x - y

# This function multiplies two numbers
def multiply(x, y):
   return x * y

# This function divides two numbers
def divide(x, y):
   return x / y

print ("Select operation.")
print ("1. Add")
print ("2. Subtract")
print ("3. Multiply")
print ("4. Divide")

choice = input("Enter choice (1/2/3/4): ")

num1 = int(input("Enter first number: "))
num2 = int(input("Enter second number: "))

if choice == '1':
   print(num1, "+", num2, "=", add(num1,num2))

elif choice == '2':
   print(num1, "-", num2, "=", subtract(num1,num2))

elif choice == '3':
   print(num1, "*", num2, "=", multiply(num1,num2))

elif choice == '4':
   print(num1, "/", num2, "=", divide(num1,num2))
else:
   print("Invalid input")

So, I would like to know whether I could make this code shorter and more efficient.

Also, any alternatives are greatly welcome.

Any help would be highly appreciated.

\$\endgroup\$
7
\$\begingroup\$
  1. You have several functions. What if you will have 100 functions? 1000? Will you copy-paste all your code dozens of times? Always keep in mind the DRY rule: "Don't repeat yourself". In your case you can store all functions and its info in some kind of structure, like dict.

  2. You run your program, it calculates something once and quits. Why not letting the user have many calculations? You can run the neverending loop with some break statement (old console programs, like in DOS, usually quitted on Q).

Here is the improved code:

# This function adds two numbers 
def add(x, y):
    return x + y

# This function subtracts two numbers 
def subtract(x, y):
    return x - y

# This function multiplies two numbers
def multiply(x, y):
    return x * y

# This function divides two numbers
def divide(x, y):
    return x / y

print("Select operation.")
print("1. Add")
print("2. Subtract")
print("3. Multiply")
print("4. Divide")

functions_dict = {
    '1': [add, '+'],
    '2': [subtract, '-'],
    '3': [multiply, '*'],
    '4': [divide, '/']
}

while True:
    choice = input("Enter choice (1/2/3/4) or 'q' to quit: ")
    if choice == 'q':
        break
    elif choice in functions_dict:
        num1 = int(input("Enter first number: "))
        num2 = int(input("Enter second number: "))
        print('{} {} {} = {}'.format(
            num1,
            functions_dict[choice][1],
            num2,
            functions_dict[choice][0](num1, num2)
        ))
    else:
        print('Invalid number')

\$\endgroup\$
  • \$\begingroup\$ I was about to post my answer and saw this. Upvote for bringing out a case for dictionaries. Might I add, it may be good to add a bit on input validation. \$\endgroup\$ – perennial_noob May 22 at 20:54
  • \$\begingroup\$ Maybe add print ("Q. Quit") after print ("4. Divide") \$\endgroup\$ – Stobor May 23 at 1:55
  • 1
    \$\begingroup\$ A few things: There should be no space after the print function. You can also use elif choice in functions_dict \$\endgroup\$ – Ben May 23 at 12:42
  • \$\begingroup\$ Thank you! elif choice in functions_dict is really better than my version. \$\endgroup\$ – vurmux May 23 at 13:13
  • \$\begingroup\$ No if main guard? \$\endgroup\$ – D. Ben Knoble May 23 at 15:38
6
\$\begingroup\$

One issue I see is with casting the user's input to int:

num1 = int(input("Enter first number: "))
num2 = int(input("Enter second number: "))

You can prompt the user the input only integers, but there is currently nothing stopping them from inputting a string. When an attempt to cast the string as an int is made, it will fail inelegantly.

I suggest either surrounding the input with a try/except block to catch that possibility:

try:
    num1 = int(input("Enter first number: "))
except ValueError:
    print("RuhRoh")

Or using str.isdigit():

num1 = input("Enter first number: ")
if not num1.isdigit():
    print("RuhRoh")
\$\endgroup\$
1
\$\begingroup\$

I think the menu is effective but a bit awkward. Reading the string directly might be a nice bit of user friendliness. The code below only multiplies two numbers, but I think it could probably go to many more.

Hopefully this code will show you about regex and such. Findall is quite useful.

    import re
    while True:
        my_operation = input("Enter a simple arithmetic operation (-+*/), no parentheses:")
        if not my_operation:
            print("Goodbye!")
            break
        numstrings = re.split("[\*\+/\-]", my_operation) #x*y, for instance
        if len(numstrings) == 1:
            print("I need an operation.")
            continue
        if len(numstrings) != 2: #2*3*4 bails
            print("I can only do a single operation right now.")
            continue
        for my_num in numstrings:
            if not my_num.isdigit(): #e.g. if you try z * 12
                print(my_num, "is not a digit.")
                continue
        numbers = [int(x) for x in numstrings] # convert strings to integers
        my_operator = re.findall("[\*\+/\-]", my_operation)[0] #this finds the first incidence of the operators
        out_string = my_operation + " = "
        if my_operator == '-': out_string += str(numbers[0] - numbers[1])
        elif my_operator == '+': out_string += str(numbers[0] + numbers[1])
        elif my_operator == '*': out_string += str(numbers[0] * numbers[1])
        elif my_operator == '/': out_string += str(numbers[0] / numbers[1])
        else: print("unknown")
        print(out_string)

Possible improvements would be to create a string r'[-+/*]' so it would be easy to add, say, 3^3 or 5%3 or even 5&3 (bitwise and) or 5|3(bitwise or).

\$\endgroup\$

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.