6
\$\begingroup\$

I've been working on this for the last few days. Any improvement ideas? (Notes are very simple, making this for school and notes have to be for complete novices to Python).

import random           # Allows the computer to generate random numbers, vital for the program to function correctly.
import sys              # Allows a new selection of system related commands to be used; I use sys.exit() to close the program.

def oneToTen():
    ten = random.randint(1,10)                     #Generates a random integer between 1 and 10.
    attempts = 1                                   
    guess = int(input("Make your guess!"))
    while guess != ten:                            #Everything indented below this will happen when the user's guess is NOT equal to the random number. != means not equal to.


        if guess < ten:                             #This will happen when the user's guess is lower than the randomly generated number.
            print("Higher!")
            attempts = attempts + 1                    #Adds one to the number of attempts the user has taken to guess the number.
            guess = int(input("Make your guess!"))

        elif guess > ten:                                    #This will happen when the user's guess is higher than the randomly generated number.
            print("Lower!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        if guess > 10:                             #This will happen when the user's guess is larger than 10.
            print("Enter a valid number!")
            guess = int(input("Make your guess!"))

        elif guess == ten:                                          #This will happen when the user's guess is equal to the random number.
            print("Well done! You guessed it in",attempts,"tries!")
            end = input()
            sys.exit()   #Exits the program.

        elif attempts == 5:
            print("The number was",ten,"!")
            end = input("Maximum amount of tries reached!")
            sys.exit()

def oneToTwenty():
    twenty = random.randint(1,20)
    attempts = 1
    guess = int(input("Make your guess!"))
    while guess != twenty:


        if guess < twenty:
            print("Higher!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        elif guess > twenty:
            print("Lower!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        if guess > 20:
            print("Enter a valid number!")
            guess = int(input("Make your guess!"))

        elif guess == twenty:
            print("Well done! You guessed it in",attempts,"tries!")
            end = input()
            sys.exit()

        elif attempts == 5:
            print("The number was",twenty,"!")
            print("Maximum amount of tries reached!")
            sys.exit()

def oneToFifty():
    fifty = random.randint(1,50)
    attempts = 1
    guess = int(input("Make your guess!"))
    while guess != fifty:


        if guess < fifty:
            print("Higher!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        elif guess > fifty:
            print("Lower!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        if guess > 50:
            print("Enter a valid number!")
            guess = int(input("Make your guess!"))

        elif guess == fifty:
            print("Well done! You guessed it in",attempts,"tries!")
            end = input()
            sys.exit()

        elif attempts == 5:
            print("The number was",fifty,"!")
            print("Maximum amount of tries reached!")
            sys.exit()

def oneToHundred():
    hundred = random.randint(1,100)
    attempts = 1
    guess = int(input("Make your guess!"))
    while guess != hundred:


        if guess < hundred:
            print("Higher!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        elif guess > hundred:
            print("Lower!")
            attempts = attempts + 1
            guess = int(input("Make your guess!"))

        if guess > 100:
            print("Enter a valid number!")
            guess = int(input("Make your guess!"))

        elif guess == hundred:
            print("Well done! You guessed it in",attempts,"tries!")
            end = input()
            sys.exit()

        elif attempts == 5:
            print("The number was",hundred,"!")
            print("Maximum amount of tries reached!")
            sys.exit()

print("Hello! In this game, you shall attempt to guess my number!")          #What the user sees upon opening the program.
print("What range would you like my number to be in?")
print("1. 1-10")
print("2. 1-20")
print("3. 1-50")
choice = int(input("4. 1-100"))          #User's choice of the range of numbers.

if choice == 1:            #Executes the code for a number between 1 and 10.
    oneToTen()

if choice == 2:            #Executes the code for a number between 1 and 20.
    oneToTwenty()

if choice == 3:            #Executes the code for a number between 1 and 50.
    oneToFifty()

if choice == 4:            #Executes the code for a number between 1 and 100.
    oneToHundred()
\$\endgroup\$
  • 3
    \$\begingroup\$ Why not simply create a function whose input is the upper point of the range that can be guessed then ask the user how high they want to guess to? Otherwise, you've created multiple identical functions which can all be compressed into one, generic function. \$\endgroup\$ – Zenohm Jul 3 '15 at 19:59
15
\$\begingroup\$

Some general comments:

  • The Python style guide recommends that lines should be wrapped to 79 characters; comments or docstrings to 72. A lot of your comments disappear of the side of the screen in my editor. Try to make your lines shorter.
  • Most Python variable names are snake_case, not camelCase, with the exception of class names, which are PascalCase.
  • Your variable names are pretty obtuse. Using ten or hundred for random integers makes the program much harder to read, because the name sounds like these are constants, not random values.
  • Your comments need some work. For example:

    attempts = attempts + 1
    #Adds one to the number of attempts the user has taken to guess the number.
    

    This is pretty much the anti-pattern for comments. Comments should explain why the code works this way, not repeat information that I could learn by reading the code.

    Most of your comments are redundant or unnecessary, and can be removed. In particular, don’t explain why you’re importing modules unless it’s particularly unusual – this just adds visual noise, and is prone to become out-of-date.

  • Your three functions oneToTen(), oneToTwenty() and oneToFifty() are all very similary. It would be better to have a single guess_number(max_number) function which asks the user to guess a number between 1 and max_number, and just call this with different parameters as appropriate, than to have all the code copying.
  • Using sys.exit() is a terrible way to yield control flow from a function, because it kills the entire script. It’s much better to use return, in case the rest of the program needs to continue.
  • If I give your program totally invalid input, it says nothing and just exits silently. It should warn me if I’ve used an inappropriate choice.
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ lowercase_with_underscore is frequently called snake_case. Also, camelCase usually implies the first letter is lowercase; what you've called CamelCase is more commonly referred to as PascalCase (see Wikipedia. \$\endgroup\$ – Aaron Dufour Jul 4 '15 at 16:17
  • \$\begingroup\$ @AaronDufour Thanks, I always get those mixed up. Fixed. \$\endgroup\$ – alexwlchan Jul 4 '15 at 17:31
11
\$\begingroup\$
  1. You have created multiple identical functions whose only difference is the name of the variable which stores the random number. These can all be compressed into a single function which has one input, the maximum for the range.

  2. Your program needs error handling to stop it from crashing if, for example, no number is entered.

  3. Nothing stops the user from entering numbers less than 1.

  4. Your comments are at points unnecessary, and are difficult to read within the code. See PEP 0008.

  5. Do not name variables after constants unless you have a very good reason. This makes the code confusing and harder to debug in the future.

  6. Consider having a message that is displayed if the correct number is displayed on the first guess.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Stuff others haven't mentioned:

You have a while loop in all your four functions. But have you noticed that the condition in all the four loops will never be false? When the user has guessed the correct number, sys.exit() executes via the elif. Remove all those sys.exit() from all the four functions.

Use if __name__ == "__main__":

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

For fun, I rewrote your program and added a little point system. You can set the max points to win the game in the global variables. I also used some of the ideas that people have said here. I created a max_rand global variable that will modify the maximum random numbers to choose from. If you like, you could set this to a user input. I also used try/except statements to keep errors from happening. I hope I gave you some good ideas with this code. If you have any questions feel free to ask me.

Note: This uses python3. if you are using python 2, replace all the inputs with raw_input, as input is for a line of code in python 2 and raw_input automagically detects the data type of the users input

# Import modules
import time
import random

# Global variables
player_input = None
player_win = False
player_points = 0
lose_points = 1
win_points = 5
max_points = 20
max_rand = 10
the_number = random.randint(1, max_rand)

# Start game
print("\n  Welcome to the number guessing game\n  If you guess the number right you get " + str(win_points) + " points.\n  If you guess the number wrong you lose " + str(lose_points) + " points.\n  To win the game you need to get " + str(max_points) + " points.\n  Have fun!\n")
start_time = time.time()

# Loops until player wins the round
while True:
    player_input = int(input("Guess the number: "))

    #Checks if player has guessed the number correctly
    if player_input > the_number:
        player_points -= lose_points
        print("\n  POINTS: " + str(player_points) + "/" + str(max_points) + " | The number is smaller than " + str(player_input) + "\n")
    elif player_input < the_number:
        player_points -= lose_points
        print("\n  POINTS: " + str(player_points) + "/" + str(max_points) + " | The number is bigger than " + str(player_input) + "\n")
    else:
        player_points += win_points
        print("\n  ************\n  * Correct! *\n  ************\n  POINTS: " + str(player_points) + "/" + str(max_points) + "\n")
        the_number = random.randint(1,10)

    # Checks if player has gained enough points to win the game
    if player_points >= max_points:
        break

# End game
elapsed_time = int(time.time() - start_time)
print("  $$$$$$$$$$$$$$$$$$$$$\n  $ You won the game! $\n  $$$$$$$$$$$$$$$$$$$$$\n  It took you " + str(elapsed_time) + " seconds!\n")
| improve this answer | |
\$\endgroup\$
  • 3
    \$\begingroup\$ Beyond using the tips in the other answers, can you explain how this improves on the OP's code? Otherwise, this is liable to be flagged as not a valid answer. \$\endgroup\$ – user34073 Jul 4 '15 at 4:40

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.