I'm a beginning coding and this is my very first Python program - a basic calculator. I would very much appreciate any recommendations or fixes!

if restart != "Y":
        print("It was a pleasure calculating with you!")
        time.sleep(1)
        print("\nGoodbye!")
        time.sleep(1)
        sys.exit()

# Modules:

import time     # Provides time-related functions
import sys      # Used to exit the program when needed


# Operations:

def add(num1, num2):
    #   Returns the sum of num1 and num2
    return num1 + num2


def sub(num1, num2):
    #   Returns the difference of num1 and num2
    return num1 - num2


def mul(num1, num2):
    #   Returns the product of num1 and num2
    return num1 * num2


def div(num1, num2):
    #   Returns the quotient of num1 and num2
    try:
        return num1 / num2
    except ZeroDivisionError:
        # Handles division by zero
        print("Division by zero cannot be done. You have broken the universe. Returning zero...")
        return 0


def exp(num1, num2):
    #   Returns the result of num1 being the base and num2 being the exponent
    return num1 ** num2


# Run operational functions:

def run_operation(operation, num1, num2):
    # Determine operationz
    if operation == int("1"):
        print("Adding...\n")
        time.sleep(1)
        print(num1, "+", num2, "=", add(num1, num2))
    elif operation == int("2"):
        print("Subtracting...\n")
        time.sleep(1)
        print(num1, "-", num2, "=", sub(num1, num2))
    elif operation == int("3"):
        print("Multiplying...\n")
        time.sleep(1)
        print(num1, "*", num2, "=", mul(num1, num2))
    elif operation == int("4"):
        print("Dividing...\n")
        time.sleep(1)
        print(num1, "/", num2, "=", div(num1, num2))
    elif operation == int("5"):
        print("Exponentiating...\n")
        time.sleep(1)
        print(num1, "^", num2, "=", exp(num1, num2))
    else:
        print("I don't understand. Please try again.")
        time.sleep(1)


def main():
    while True:                   # Loops if user would like to restart program
        valid_input = False
        while not valid_input:    # Loops if user enters an invalid input. Avoids error
            try:
                # Acquire user input
                num1 = (int(input("What is number 1? ")))
                num2 = (int(input("What is number 2? ")))
                operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, "4. Division, 5. Exponentiation \nPlease choose an operation: "))
                valid_input = True
            except ValueError:      # Handles any value errors
                print("Invalid input. Please try again.")
                time.sleep(1)
        run_operation(operation, num1, num2)
        time.sleep(1)
        # Ask if the user wants to do more calculations or exit:
        restart = (str(input("Would you like to make more calculations? Enter (Y) for yes, or any other character for no. ")))

I'm having a lot of trouble with the code block below. It doesn't recognize "Y" and instead outputs: "It was a pleasure calculating with you! Goodbye!" What could I do?

if restart != "Y":
    print("It was a pleasure calculating with you!")
    time.sleep(1)
    print("\nGoodbye!")
    time.sleep(1)
    sys.exit()


#  Title Sequence
print('\n\nCalculator\n\n')
time.sleep(1)
print('      ---LOADING---\n\n')
time.sleep(2)
print('Hello. I am your personal calculator. \nBEEP BOOP BEEP. \n\n')
time.sleep(1)


#  Calls the main function to start program
main()

# Turns my program into an importable module
if __name__ == '__calculator__':
    main()

closed as off-topic by 200_success Dec 18 '17 at 16:47

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 7
    A quick note is that if your code doesn't work it probably belongs over in Stack overflow – 13ros27 Dec 18 '17 at 7:45
  • 3
    Just so you know, it's considered bad practice to edit your question with your new code, because it makes the answers obsolete. If you have another question, ask a whole new question either here or on SO. – OldBunny2800 Dec 18 '17 at 15:36
  • 3
    I have rolled back Rev 2 → 1. As per our rules, code cannot be edited after answers have been posted. In this case, this question should never have been asked or answered on Code Review, because it was obviously not working as intended. Once you have a program that works fully as intended, you can post it as a new question. – 200_success Dec 18 '17 at 16:51

Thanks for asking your question! I'll try and give a concise answer to the main issues with your code. Overall it is well done for a beginner.

First, I'll address the code block you're having trouble with. The restart variable is never defined, so I'm not even sure how your code ran at all. You will need to give restart some initial value, supposedly "Y" as that would have it skip over that code in the beginning.

There is a bigger problem here though, which is that you are putting code in the global scope and also in a main function. Usually, Python scripts do one or the other, as they mean different things to other Python programmers. Although it is completely valid what you're doing, it's not good Python. The purpose of the main function and the if __name__ == '__main__' conditional is so that you can separate the "executable" part of your program from the "library" part.

(See this StackOverflow answer for more detail: https://stackoverflow.com/questions/419163/what-does-if-name-main-do)

The main function allows your code to have different behavior when your program is run on its own, as opposed to when it's imported and usually this means that the imported code does not run anything on its own especially the blocks you're having trouble with.

I would suggest putting all the "running" code, that is, the restart check, and the "calculator startup" all in the main function. And, if you want to logically separate these ideas, then put them all in separate functions that run from main. Something perhaps along the following lines:

def restart(response):
    # uses "in" to check multiple values,
    # a replacement for (response == "Y" or response == "y")
    # which is longer and harder to read.
    if response in ("Y", "y"):
        return True
    else:
        # statements you want to print to the screen before exiting
        ...
        return False

And in the main function:

def main():
    ...
    setup_calculator()  # something you would write, feel free to give it a better name
    ...
    restart_msg = "Would you like to make more calculations?" \
                  "Enter (Y) for yes, or any other character for no. "
    if not restart(str(input(restart_msg))):  # uses the function I wrote
        return  # would not need to import "sys", just exit from main normally

You can do a similar thing for the calculator startup, but I'll leave that to you.

In your run_operation function, you write conditionals with int("1"), int("2"), ... etc. You can just write 1, 2, ... etc, for each if statement. Also, you don't have to worry about typecasting the inputs to run_operation because you do that in the input in main. Once you have the types from the user input, assume that your code runs on that specific type.

One more thing I noticed is that almost everytime you print to the console, you also perform a time.sleep(1), (in one case a time.sleep(2)). What I suggest, so that you don't repeat yourself, is to write your own print function that also performs a sleep every time,

def calculator_print(*args, delay=1):
    print(*args)
    time.sleep(delay)

You could give it a better name, I'm just concerned with the form that it has. This would probably halve your line count and improve readability by many times. The *args is a way for you to accept any number of arguments to calculator_print to match the way that print itself can and does accept many arguments, like in your run_operation function. The delay default argument is there to set the sleep delay to a different value other than 1 like in your calculator startup. This will get rid of all of your explicit time.sleep calls and abstracts away the details of the printing itself. (Say you wanted to change it to do something different, you can still use the rest of the code you've written in the same way. You would only have to change the calculator_print function. Encapsulation is good!)

If you fix these few problems, I think you will be rid of any bugs that you have already and your readability will skyrocket. A very very important part of programming that is grossly overlooked is readability. This should always be taught alongside syntax. As a programmer you will be reading code more than writing it so you should be good at reading good (and bad) code and writing code that makes it easier for others to read; you may even be reading your own old code one day. Writing comments and clear code are both essential for readability.

You've done a really good job here as a beginner. Let me know if you have any questions about what I've said or about anything else Python.

Keep up the good work!

  • @RichardYudkiss you have input twice in your final code, once with restart_msg = input("...") and again in the restart method call. It is only necessary in one place. (sorry I can't comment on your post directly) – Brandon Gomes Dec 18 '17 at 6:49
  • Why do you call str(input(...)) as opposed to just input()? – Solomon Ucko Jun 27 at 21:08

You can also avoid creating functions to compute the results and using what operator module from the standard library offers. You can create a list of operation symbols together with the operator functions first. Why list - because then we are going to map the user operation number choices to the positions/indexes in this list:

import operator


OPERATIONS = [
    ("+", operator.add),
    ("-", operator.sub),
    ("*", operator.mul),
    ("/", operator.div),
    ("**", operator.pow)
]

Then, you can map the user inputs to the functions and call them:

def run_operation(operation, num1, num2):
    if operation >= len(OPERATIONS):
        print("I don't understand. Please try again.")
        time.sleep(1)
        return

    symbol, operation = OPERATIONS[operation - 1]
    time.sleep(1)
    print(num1, symbol, num2, "=", operation(num1, num2))
  1. There is a syntax error as following due to extra double quotes:

    operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, "4. Division, 5. Exponentiation \nPlease choose an operation: "))
    

    which I changed to:

    operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, 4. Division, 5. Exponentiation \nPlease choose an operation: "))
    
  2. The restart variable is not defined. The issue is that only the code in the main method gets executed. I changed the code to:

    def main():
        while True:                   # Loops if user would like to restart program
            valid_input = False
            while not valid_input:    # Loops if user enters an invalid input. Avoids error
                try:
                    # Acquire user input
                    num1 = (int(input("What is number 1? ")))
                    num2 = (int(input("What is number 2? ")))
                    operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, 4. Division, 5. Exponentiation \nPlease choose an operation: "))
                    valid_input = True
                except ValueError:      # Handles any value errors
                    print("Invalid input. Please try again.")
                    time.sleep(1)
            run_operation(operation, num1, num2)
            time.sleep(1)
            # Ask if the user wants to do more calculations or exit:
            restart = raw_input("Would you like to make more calculations? Enter (Y) for yes, or any other character for no. ")
            if restart != "Y":
                print("It was a pleasure calculating with you!")
                time.sleep(1)
                print("\nGoodbye!")
                time.sleep(1)
                sys.exit()
    
    
            #  Title Sequence
            print('\n\nCalculator\n\n')
            time.sleep(1)
            print('      ---LOADING---\n\n')
            time.sleep(2)
            print('Hello. I am your personal calculator. \nBEEP BOOP BEEP. \n\n')
            time.sleep(1)
    
    main()
    
    # Turns my program into an importable module
    if __name__ == '__calculator__':
        main()
    
  3. The code would never enter the except part as it never is going to catch the exception. It should be Namerror if you wanted to give an option for the user to retry in case he entered a non-integer:

        while not valid_input:    # Loops if user enters an invalid input. Avoids error
        try:
            # Acquire user input
            num1 = (int(input("What is number 1? ")))
            num2 = (int(input("What is number 2? ")))
            operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, 4. Division, 5. Exponentiation \nPlease choose an operation: "))
            valid_input = True
        except NameError:      # Handles any value errors
            print("Invalid input. Please try again.")
            time.sleep(1)
    
  4. The conversion of int("1") etc. is unnecessary. You could do:

    operation == 1
    
  1. The repetition of input() at the end of the program.

        restart_msg = input("Would you like to do more calculations? Enter (Y) for yes, or any "
                        "other character for no. ")
    if not restart(str(input(restart_msg))):
        return
    

    restart_msg already asks the user for input, asking again confuses the user. I'd change it to:

    restart_msg = input("Would you like to do more calculations? Enter (Y) for yes, or any "
                 "other character for no. ")
    

    if not restart(restart_msg): # uses the function I wrote return Note: input() interprets everything typed in as a string by default, having str(input()) isn't necessary.

  2. ValueError when typing "y" into a number input.
    Your current screenshot has a ValueError: invalid literal for int() with base 10: 'Y' error, caused by the line num1 = (int(input("What is number 1? "))).
    int() takes a string containing numerical symbols e.g. "1234" and turns it into integers you can do maths on, e.g. 1234. If you feed it something else, like "y", it will return a ValueError. To catch this error, we can just add another exception to the try except. You can add multiple errors per except by putting the errors in a tuple.

        operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, "
                              "4. Division, 5. Exponentiation \nPlease choose an operation: "))
        valid_input = True
    except (NameError, ValueError):  # Handles any value errors
        calculator_print("Invalid input. Please try again.")
    

Nitpicking: You don't need valid_input. I'd use the continue keyword.
continue moves the program back to the top of the current loop. Because of this, we can cut out if not valid_input:, and we don't need to redefine the variable every time it loops. To implement this, make the program run normally, and if an exception is raised, return to the top of the loop:

while True:  # Loops if user would like to restart program
    try:
        # Acquire user input
        num1 = (int(input("What is number 1? ")))
        num2 = (int(input("What is number 2? ")))
        operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, "
                              "4. Division, 5. Exponentiation \nPlease choose an operation: "))
    except (NameError, ValueError):  # Handles any value errors
        calculator_print("Invalid input. Please try again.")
        continue
    run_operation(operation, num1, num2)
    #...

Hope this helps solve the problems you've been facing. This is my first StackExchange post, so forgive any inconsistencies and/or rule breaking.

  • I find your way of handling division by zero strange. Division by zero is undefined, not \$ 0 \$. I'd expect the exception to be propagated:

    def div(num1, num2):
        return num1 / num2
        # A ZeroDivisionError may occur here, but the traceback is straightforward
    

    Or use a custom error message:

    def div(num1, num2):
        try:
            return num1 / num2
        except ZeroDivisionError:
            raise ZeroDivisionError("Operand 2 cannot be zero. Division by zero is undefined.")
    
  • Docstrings are a great way to help others understand what your code does. Classes and functions often have docstrings (except if they are internal). A docstring is simply a string literal and corresponds to an object's __doc__ attribute.

    Examples:

    def add(num1, num2):
        """Return the sum of num1 and num2."""
        return num1 + num2
    

    With argument descriptions:

    def sub(num1, num2):
        """Return the difference of num1 and num2.
        Arguments:
        - num1: integer / float: The first operand.
        - num2: integer / float: The second operand.
        """
        return num1 - num2
    

    You may enforce your own docstring style, but take a look at PEP-257 for some recommendations.

  • You don't have to include the "\n" in the print statement, since print() ends with a newline by default.

  • Frankly, I don't see the point in calculator_print() at all. The 'turing feel' (?) will become very annoying to end users. IMO you should scrap it.

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