I've been learning Python for about 4 days now. Python is my first programming language, so I'm not that good at coding yet. I have been programming a little random number game as my first project and came to a state of code with which I'm kind of happy.
My code does what I want: it generates a random number between 1 and 20, prints the initial tip and asks the user for an input guess. Should I guess right it completes the round, adds 100 points to my score and continues with round 2. When I do not guess correctly, it subtracts 20 points off my score and asks me to try again for another 2 attempts until I lose.
In which ways can I improve my code? What are the weaknesses of this code and what could be ways to make code more efficient, clearer and generally better?
import random
attempt = 1
user_input = "ab"
class Test:
def __init__(self):
self.score = 100
self.testguess = 0
self.y = len("yes")
self.z = len("no")
self.number_holder = 0
def number_gen(self):
self.number_holder = random.randrange(1,20)
def first_tip(self):
if(self.number_holder % 2 == 0 and self.number_holder <= 10):
print("*Your number is even and smaller than 10 or the same")
elif(self.number_holder % 2 == 0 and self.number_holder >= 10):
print("*Your number is even and bigger than 10 or the same")
elif(self.number_holder % 2 != 0 and self.number_holder <= 10):
print("*Your number is uneven and smaller than 10 or the same")
elif(self.number_holder % 2 != 0 and self.number_holder >= 10):
print("*Your number is uneven and bigger than 10 or the same")
print("*************************************")
print(f"*{self.number_holder}")
def make_try(self):
print("*************************************")
print("*My guess would be:")
print("*************************************")
self.testguess = int(input("*"))
def check_win(self, attempt):
if(self.number_holder == self.testguess):
self.score = self.score
print("*************************************")
print(f"*You have won, your final score was: {self.score}")
print("*************************************")
return True
elif(self.number_holder != self.testguess and self.testguess != 0):
attempt_calc = 3 - attempt
self.score = self.score - 20
print("*************************************")
print(f"*That was not correct. Your new score: {self.score}")
print("*************************************")
print(f"*You have {attempt_calc} attempts left.")
return False
def start_again(self, user_input, attempt):
x = len(user_input)
if(x == self.y):
return True
elif(x == self.z):
return False
else:
print("*************************************")
print("*Wrong input")
print("*Do you want to play the NumberGame?")
print("*Type Yes/yes or No/no")
print("*************************************")
user_input = input("*")
if __name__ == "__main__":
T1 = Test()
for i in range(3):
print(i)
T1.number_gen()
while attempt <= 3 and user_input != "no" and user_input != "No":
if(attempt == 1):
print("*************************************")
print("*Do you want to play the NumberGame?[Yes/No]")
print("*************************************")
user_input = input("*")
("*************************************")
elif(attempt == 2):
print("*************************************")
print("*Do you want to try again?[Yes/No]")
print("*************************************")
user_input = input("*")
print("*************************************")
elif(attempt == 3):
print("*************************************")
print("*Do you want to try again?[Yes/No]")
print("*************************************")
user_input = input("*")
print("*************************************")
if(T1.start_again(user_input, attempt) == False):
print("*************************************")
print("*See you next Time!")
print("*************************************")
break
if(attempt == 1):
T1.first_tip()
T1.make_try()
if(T1.check_win(attempt) == True):
break
elif(attempt == 3):
print("*************************************")
print("*No more attempts.")
print("*************************************")
break
attempt = attempt + 1
print("*************************************")
print("*Thanks for playing")
print("*************************************")
attempt = 1
2 Answers 2
- Do not call your class Test - it does not perform tests, does it?
- You are initializing some variables (notably,
attempt
) outside of the class responsible for handling the game state. Make this logic cohesive: the game state should have clear interfaces (taking a guess, trying again...). And the way you are initializinguser_input
makes it compare to "no" in your logic. Speaking of which... self.y = len("yes")
is a cardinal sin. Not only it is a very backwards way to assign a variable with nonspecific name a value of 3, it is also a fairly weird way to test the user input for "yes" or "no". A more common way to do so if it is case sensitivity that you are concerned with is something likeinput().lower() == 'yes'
orinput().lower() in ['yes', 'y']
, if you want to allow even more options.first_tip
could and, arguably, should be streamlined:
parity_string = 'even' if self.number_holder % 2 == 0 else 'odd'
order_string = 'less' if self.number_holder <= 10 else 'greater'
print(f"*Your number is {parity_string} and {order_string} than or equal to 10")
Also, um, it gives away the whole game at the end of the method: print(f"*{self.number_holder}")
check_win
has anif
andelif
clauses; the checkself.number_holder != self.testguess
is completely redundant in the latter and, more importantly, the control flow becomes a mess there: you do not explicitly tackle the case when elif condition is not satisfied (e.g. user inputs 0), so that it returns None and the main loop continues.Related to the previous issue: you handle some of the game logic in its own class, and some of it in
__main__
, while essentially all it does in both cases is reading the user input and giving some responses.start_again
seems to silently make an assumption the user will comply the second time and actually type "yes" or "no". Furthermore, it can and will returnNone
if the user messes up the first time (not to mention these length comparisons I wrote about above), and this branch will have a side effect on the loop condition (user_input
will be modified and checked in theuser_input != "no"
bit) - this makes the overall logic quite opaque. And you are not using the attempt variable in this function at all - why have it there, then?Minor stylistic issues - parentheses in your if statements are completely redundant,
self.score = self.score
is hardly a useful statement. Neither isT1.check_win(attempt) == True
(gettingTrue
fromTrue == True
is bit weird: it is already true!).
So, the main direction that needs to be taken to improve this code is to address the control flow and make the logic of what happens under which conditions more transparent. Another thing to keep in mind - you are using the user input for both controlling the flow of the game (retrying) and guessing the numbers. In its current state, it makes little sense to store it at all, but you might want to store previous guesses for whatever reason: keep them separate from the input pertinent to the decisions to keep playing.
A bit cleaned up version:
import random
class Game:
def __init__(self):
self.score = 100
self.remaining_attempts = None
self.user_guess = None
self.number_holder = None
self.generate_state()
def generate_state(self):
self.remaining_attempts = 3
self.number_holder = random.randrange(1,20)
def give_hint(self):
parity_string = 'even' if self.number_holder % 2 == 0 else 'odd'
order_string = 'smaller' if self.number_holder <= 10 else 'larger'
print(f"*Your number is {parity_string} and {order_string} than or equal to 10")
def user_wants_to_continue(self, bad_input=False):
if self.remaining_attempts == 3: # Be careful here! Possibly decouple the current attempt from remaining
prompt = 'Do you want to play the NumberGame?[Yes/No]'
else:
prompt = 'Do you want to try again?[Yes/No]'
print('*************************************')
if bad_input:
print('Sorry, try again')
print(prompt)
print('*************************************')
user_input = input("*")
if user_input.lower() not in ['yes', 'no']:
return self.user_wants_to_continue(bad_input=True) # Recursive call beating user into submission
else:
return user_input.lower() == 'yes'
def make_attempt(self):
print("*************************************")
print("*My guess would be:")
print("*************************************")
self.user_guess = int(input("*"))
self.remaining_attempts -= 1
def check_win(self):
if(self.number_holder == self.user_guess):
print("*************************************")
print(f"*Congratulations, you have won! Your final score was: {self.score}")
print("*************************************")
return True
else:
self.score -= 20
print("*************************************")
print(f"*That was not correct. Your new score: {self.score}")
print("*************************************")
print(f"*You have {self.remaining_attempts} attempts left.")
return False
def play(self):
user_input = ''
for i in range(3):
self.generate_state()
while self.user_wants_to_continue():
if self.remaining_attempts > 0:
if self.remaining_attempts == 3:
self.give_hint()
self.make_attempt()
if self.check_win():
break
else:
print("*************************************")
print("*No more attempts.")
print("*************************************")
break
print("*************************************")
print("*Thanks for playing")
print("*************************************")
I might have changed the logic a little bit, particularly, the fixed loop of 3 games bothers me. But I was not sure about the original intent there.
-
1\$\begingroup\$ Thank you fpr taking the time to review my code :) I will redo my code with your tips in mind and hopefully improve it. \$\endgroup\$Marc Matthäus– Marc Matthäus2022年03月07日 13:48:45 +00:00Commented Mar 7, 2022 at 13:48
As a beginner, focus your attention on building programs with ordinary
functions. You built this guessing game using a class. In the abstract, that
might be a sensible approach, but you're a raw beginner -- not just at Python
but at coding. And the bread-and-butter of effective coding is learning how to
break down a problem into small components, each able to perform a narrowly
defined task. The best way to gain such skills is by building programs with
simple functions. Adding object-oriented code into the mix at this stage is
mostly a distraction and often leads to bloated, over-engineered solutions. It
also tends to prolong one of the most common beginner bad habits (a reliance on
global variables), with expansive classes (such as your Test
) becoming a
vehicle to smuggle global information back into the mix.
Start with a pseudo-code plan to play the game once. Your game roughly operates like this:
Does the user want to play?
If no:
exit with goodbye message
secret = generate random number
for attempt in (1, 2, 3)
if attempt == 1:
give hint
guess = get guess from user
if guess == secret:
exit with success message
exit with failure message
Create a top-level rough draft of the code, organized around functions. The next step is to convert that logical plan into functions. Functions should be focused: they should do one thing, not everything. Functions should be data-oriented whenever feasible: take data as arguments, return data as results. And functions should be generalized whenever that's helpful to us: for example, if we know we need to ask the user multiple yes/no questions, try to build that behavior once (in a generalized way) rather than multiple times (each with its own particulars); or, to give an example from your current code, if we want to print a headline-banner message we could write a function taking a message and do the printing of the asterisks in just one spot rather repeating those print statements dozens of times. On the first draft, it's alright to take major shortcuts: for example, we might start by skipping the implementation of the needed helper functions. Here's what the game-playing function might look like:
def number_game():
if not yesno('Want to play the number game'):
exit_game('Goodbye')
secret_range = range(1, 21)
secret = random.choice(secret_range)
for attempt in range(1, 4):
if attempt == 1:
print(get_hint(secret))
guess = get_guess(secret_range)
if guess == secret:
exit_game('You won')
exit_game('You lost')
if __name__ == '__main__':
number_game()
Filling in the gaps: helper functions. So far, we have a function to play a single game, but we are missing the lower-level functions. I'll leave the helper functions to you, but I'll offer one suggestion. People make mistakes so you should give your user the ability to recover from data-entry errors and you should validate their inputs: for example, if you ask for yes/no, make sure you get a 'yes' or a 'no'; or if you ask for an integer from 1 through 20, make sure you get one. The most flexible way to do that is with a while-true loop: get the user response; check it for validity; if OK, return the converted/validated value; otherwise, stay in the while-true loop and ask again.
def yesno(prompt):
# Return True or False.
while True:
...
def get_guess(secret_range):
# Return an int in the range.
while True:
...
def get_hint(secret):
# Return a message.
...
def exit_game(message):
print(message)
exit()
Filling in the gaps: the ability to play more than once. If you've
implemented the ability to play once using well-designed functions, switching
to playing multiple times is usually easy. (1) We add a top-level function to
ask whether the user wants to play again. (2) We convert number_game()
into a
data-oriented function that returns true/false rather than a function that exits.
def main():
while yesno('Want to play the number game'):
player_won = number_game()
print('You won' if player_won else 'You lost')
exit_game('Goodbye')
def number_game():
secret_range = range(1, 21)
secret = random.choice(secret_range)
for attempt in range(1, 4):
if attempt == 1:
print(get_hint(secret))
guess = get_guess(secret_range)
if guess == secret:
return True
return False
if __name__ == '__main__':
main()
Functions are flexible. As illustrated in
the previous point, the approach suggested above is flexible for future enhancements: for example, we could tally up
wins/losses and report that at the end of main()
; we could take command-line
arguments to let the user control the size of secret_range
; we
could enhance the get_hint()
logic to tell the user whether their
guess was too low or high. Such flexibility comes from the focus
on organizing the program around small data-oriented functions.
-
1\$\begingroup\$ I thank you a lot for for taking the time to review my code :) I will work on it again with your tips in mind. \$\endgroup\$Marc Matthäus– Marc Matthäus2022年03月07日 13:41:34 +00:00Commented Mar 7, 2022 at 13:41
Explore related questions
See similar questions with these tags.