patternpythonMinor
Beginner - Guessing Game
Viewed 0 times
beginnergameguessing
Problem
In the past few days i have started to learn python 3 in my spare time and i am really enjoying it so far.
I have attempted many simple programs such as a palindrome checker and reverse a string in an attempt to improve my skills.
I was hoping that someone would be able to provide some feedback on the following program which is a guessing game program. The user is able to choose the number of guesses and the max possible value.
From my own perspective i think that possibly i am relying too much on while loops and if/else statements. Also i am not sure if my comments are sufficient, do they lack detail?
Also i use parentheses around the conditionals in my if/else statements and in my while loops as i find it easier to read the condition this way. Is this a bad idea? Could it confuse people who are trying to read my code?
```
# In this program, the user must try to guess a random number generated by the computer
from random import randint
def guess_game():
replay = 1
#Replay allows a user to replay the game without having to call the function again.
#The values are: 1 = Replay, any other character = End program
while (replay == 1):
#lives = Number of chances you have to correctly guess the answer before the program finishes
lives = int( input("How many lives would you like? ") )
#max_value = upper bound for the randomly generated number. Starts at 0
max_value = int( input("Please choose the largest number that can be generated: ") )
random = randint(0, max_value) #Generates a random int between 0 and max_value
num_of_guesses = 1 #keeps track of the number of guesses made by the user
while (num_of_guesses lives):
print("You have run out of guesses. The correct answer was:", random)
replay = int( input("\nWould you like to play again? Enter 1 for yes, any other character for no: ") )
if (replay != 1):
print("Thank you for playing. See you n
I have attempted many simple programs such as a palindrome checker and reverse a string in an attempt to improve my skills.
I was hoping that someone would be able to provide some feedback on the following program which is a guessing game program. The user is able to choose the number of guesses and the max possible value.
From my own perspective i think that possibly i am relying too much on while loops and if/else statements. Also i am not sure if my comments are sufficient, do they lack detail?
Also i use parentheses around the conditionals in my if/else statements and in my while loops as i find it easier to read the condition this way. Is this a bad idea? Could it confuse people who are trying to read my code?
```
# In this program, the user must try to guess a random number generated by the computer
from random import randint
def guess_game():
replay = 1
#Replay allows a user to replay the game without having to call the function again.
#The values are: 1 = Replay, any other character = End program
while (replay == 1):
#lives = Number of chances you have to correctly guess the answer before the program finishes
lives = int( input("How many lives would you like? ") )
#max_value = upper bound for the randomly generated number. Starts at 0
max_value = int( input("Please choose the largest number that can be generated: ") )
random = randint(0, max_value) #Generates a random int between 0 and max_value
num_of_guesses = 1 #keeps track of the number of guesses made by the user
while (num_of_guesses lives):
print("You have run out of guesses. The correct answer was:", random)
replay = int( input("\nWould you like to play again? Enter 1 for yes, any other character for no: ") )
if (replay != 1):
print("Thank you for playing. See you n
Solution
Functions
Everything is in one function. You should stick to the single responsibility principle. Basically, a function should have one job.
In a way, that does mean your whole game should be in the function, but it'd be better to have parts of it separated out. For example, getting an
Now this may seem unnecessary, but now you could add input validation. Caridorc pointed out a bug, so you should do error handling. The best way to do this in Python is to catch any
Now the code wont crash when an invalid string is entered, but it also wont return a value. So you want to loop over this with
Now you have error handling set up, but you can still use one line to get each value:
Pythonic style
While we're on
Also you don't need brackets around the condition, Python looks nicer without them and evaluates it just fine.
Instead of using
use Python's
In fact, instead of that at all, you could just use a for loop instead of a
You could also make use of Python's
Readability
You have good names here. They're clear and readable, without being too long or overlapping. Although, I would advise against using
However you have a lot of comments that don't need to be here. For example:
This is quite a verbose comment. At most I would have
Everything is in one function. You should stick to the single responsibility principle. Basically, a function should have one job.
In a way, that does mean your whole game should be in the function, but it'd be better to have parts of it separated out. For example, getting an
int from the user could be a function:def get_int(message)
return int(input(message))
lives = get_int("How many lives would you like? ")
max_value = get_int("Please choose the largest number that can be generated: ")Now this may seem unnecessary, but now you could add input validation. Caridorc pointed out a bug, so you should do error handling. The best way to do this in Python is to catch any
ValueErrors that get thrown.try:
return int(input(message))
except ValueError:
print("Please enter an integer only")Now the code wont crash when an invalid string is entered, but it also wont return a value. So you want to loop over this with
while until a valid number is entered and returned, ending your loop. This is how it would look:def get_int(message)
while True:
try:
return int(input(message))
except ValueError:
print("Please enter an integer only")Now you have error handling set up, but you can still use one line to get each value:
lives = get_int("How many lives would you like? ")
max_value = get_int("Please choose the largest number that can be generated: ")Pythonic style
While we're on
while, there's a more Pythonic way to do your main game loop. Instead of using replay, just use another infinite loop with while True and end your loop with a break, like this:if replay != 1:
print("Thank you for playing. See you next time")
breakAlso you don't need brackets around the condition, Python looks nicer without them and evaluates it just fine.
Instead of using
num_of_guesses = num_of_guesses + 1use Python's
+= operator:num_of_guesses += 1In fact, instead of that at all, you could just use a for loop instead of a
while loop here:for num_of_guesses in range(1, lives + 1):
guess = get_int("Please enter your guess: ")
if guess == random:
print("Good guess, that is correct! You got the right answer on guess number", num_of_guesses, ", well done!\n")
break #if answer is correct this will exit the while loop
else:
print("Unlucky, that guess was incorrect. You have", (lives - num_of_guesses), "lives remaining.\n")
if num_of_guesses > lives:
print("You have run out of guesses. The correct answer was:", random)range lets you just loop over a list of numbers. In this case, it loops from one up to lives. Range will iterate up to but not include the second parameter, which means if you pass lives + 1 the last loop will be on the value of lives.You could also make use of Python's
else clause for for loops. An else after a for loop in Python will be executed if the loop fully runs it's course without reaching a break command. If break gets called then the else block is skipped. This works perfect for your case, so I'd write it like this:for num_of_guesses in range(1, lives + 1):
guess = get_int("Please enter your guess: ")
if guess == random:
print("Good guess, that is correct! You got the right answer on guess number", num_of_guesses, ", well done!\n")
break #if answer is correct this will exit the while loop
else:
print("Unlucky, that guess was incorrect. You have", (lives - num_of_guesses), "lives remaining.\n")
else:
print("You have run out of guesses. The correct answer was:", random)Readability
You have good names here. They're clear and readable, without being too long or overlapping. Although, I would advise against using
random as a variable name. It doesn't cause a clash here since you used from random import randint, but it causes mild confusion. Something like answer would actually be more useful for indicating what the number is for.However you have a lot of comments that don't need to be here. For example:
break #if answer is correct this will exit the while loopThis is quite a verbose comment. At most I would have
# Correct answer. People reading this code probably know how break works. Your comments should focus on intent and abstract meaning. Python programmers can read your code and know what each part does, but why it does it, and what you want to do that for are useful to know in comment form. (as a beginner, perhaps you want to remind yourself of what these things do. This may be of benefit to start with, but I suggest reducing them as soon as you feel comfortable with it).Code Snippets
def get_int(message)
return int(input(message))
lives = get_int("How many lives would you like? ")
max_value = get_int("Please choose the largest number that can be generated: ")try:
return int(input(message))
except ValueError:
print("Please enter an integer only")def get_int(message)
while True:
try:
return int(input(message))
except ValueError:
print("Please enter an integer only")lives = get_int("How many lives would you like? ")
max_value = get_int("Please choose the largest number that can be generated: ")if replay != 1:
print("Thank you for playing. See you next time")
breakContext
StackExchange Code Review Q#116885, answer score: 6
Revisions (0)
No revisions yet.