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\$
0

3 Answers 3

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\$
5
  • \$\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\$ May 22, 2019 at 20:54
  • \$\begingroup\$ Maybe add print ("Q. Quit") after print ("4. Divide") \$\endgroup\$
    – Stobor
    May 23, 2019 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, 2019 at 12:42
  • \$\begingroup\$ Thank you! elif choice in functions_dict is really better than my version. \$\endgroup\$
    – vurmux
    May 23, 2019 at 13:13
  • \$\begingroup\$ No if main guard? \$\endgroup\$ May 23, 2019 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 and acknowledge that you have read and understand our privacy policy and code of conduct.

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