Recently, I got into programming with Python as a fun little side hobby. After watching a bunch of YouTube tutorials (to learn about IDE's, variables, etc.), I finally got to the point where I was able to make this really simple game where the player tries to guess the number that the computer generated. So, today I was hoping to get some feedback on the code I wrote.
I'm mostly interested in learning about the best practices for this language, but any criticism is welcome:
player.py
class Player:
def __init__(self):
"""Initializes the player object's attributes."""
self.username = "Player_1"
self.guess = None
self.wins = 0
def create_username(self):
"""Prompts the player to enter their username."""
self.username = input("What is your name?: ")
def get_username(self):
"""Returns the player's username as a string."""
return self.username
def get_guess(self):
"""Returns the player's guess as an integer."""
is_valid_input = False
while not is_valid_input:
try:
self.guess = int(input("Can you guess what it is?: "))
is_valid_input = True
return self.guess
except ValueError:
print()
print("Please enter a whole number.")
def add_win(self):
"""Adds a win to the player's win count."""
self.wins += 1
def get_wins(self):
"""Returns the player's win count as a string."""
return str(self.wins)
random_number_guesser.py
from player import Player
import random
class Random_Number_Guesser:
def __init__(self):
"""Initializes the game object's attributes."""
self.player = None
self.winning_number = None
def create_player(self):
"""Creates a new player object."""
self.player = Player()
self.player.create_username()
def greet_player(self):
"""Greets the player."""
print("Welcome, " + self.player.get_username() + "!")
def generate_random_number(self):
"""Generates a winning number."""
self.winning_number = random.randint(1, 10)
print()
print("The winning number is: " + str(self.winning_number) + ".")
print("I'm thinking of a number between 1 & 10.")
def guess_and_check(self):
"""Compares the player's guess to the winning number."""
# Checks if the player's guess is correct.
if self.player.get_guess() == self.winning_number:
print()
print(self.player.get_username() + " Won!")
self.player.add_win()
else:
print()
print(self.player.get_username() + " Lost!")
def play_again(self):
"""Determines whether or not the player wants to play again."""
is_finished_playing = False
while not is_finished_playing:
play_again = input("Would you like to play again? (y/n): ")
# Checks if the player wants to play again & handles invalid inputs.
if play_again == "y":
self.generate_random_number()
self.guess_and_check()
elif play_again == "n":
print()
# Checks the player's win count and determines if 'time' or 'times' should be used.
if int(self.player.get_wins()) > 1 or int(self.player.get_wins()) == 0:
print(self.player.get_username() + " won " + self.player.get_wins() + " times!")
else:
print(self.player.get_username() + " won " + self.player.get_wins() + " time!")
break
else:
print()
print("Please enter either 'y' or 'n'.")
def play_random_number_guesser():
print("Welcome To Random Number Guesser!")
print("=================================")
game = Random_Number_Guesser()
game.create_player()
game.greet_player()
game.generate_random_number()
game.guess_and_check()
game.play_again()
game.py
from random_number_guesser import *
if __name__ == "__main__":
play_random_number_guesser()
1 Answer 1
It's a good start.
The good
You've avoided global variables.
You've written properly-formatted docstrings.
You have multiple modules (fancy!)
You have some input validation.
You have a correctly-defined __main__
guard and top-level method.
Broadly, the code does seem to function as intended.
The bad
username
mutates, when it shouldn't need to - it can be passed into the constructor and then never changed. Decreasing member mutation will create more provably-correct and more easily-testable code.
Do not store self.guess
as a member.
Add type hints. In this case, since you have very few functions that accept arguments or return values, almost everything is None
.
It's conventional for Python class members to be accessed directly instead of through getters, so delete get_username
etc.
get_username
in particular is better represented as a __str__
method so that you can write the likes of print(player)
and it will do the right thing.
You need to get out of the habit of writing unnecessary loop variables such as is_valid_input
. Many such variables can be replaced with a simple break
when the time comes.
Since this is in theory a game, you should delete your The winning number is:
debug statement.
generate_random_number
only needs to be called from one place in this program - in the middle of the play_again
loop.
In that play_again
method, you actually need two loops - an outer loop for the game, and an inner loop for input validation.
Your plural check can be simplified by checking for equality with 1.
1 and 10 should not be hard-coded; move them to class member variables or statics.
The ugly
Don't add underscores to your class names; PEP8 asks for UpperCamelCase.
Suggested
Certainly other things to improve, but this is a start:
import random
class Player:
def __init__(self) -> None:
"""Initializes the player object's attributes."""
self.username = "Player_1"
self.wins = 0
def create_username(self) -> None:
"""Prompts the player to enter their username."""
self.username = input("What is your name?: ")
def get_guess(self) -> int:
"""Gets the player's guess as an integer."""
while True:
try:
return int(input("Can you guess what it is?: "))
except ValueError:
print("Please enter a whole number.")
def add_win(self) -> None:
"""Adds a win to the player's win count."""
self.wins += 1
def __str__(self) -> str:
return self.username
class RandomNumberGuesser:
MIN = 1
MAX = 10
def __init__(self) -> None:
"""Initializes the game object's attributes."""
self.player: Player | None = None
self.winning_number: int | None = None
def create_player(self) -> None:
"""Creates a new player object."""
self.player = Player()
self.player.create_username()
def greet_player(self) -> None:
"""Greets the player."""
print(f"Welcome, {self.player}!")
def generate_random_number(self) -> None:
"""Generates a winning number."""
self.winning_number = random.randint(self.MIN, self.MAX)
# Debug only
# print(f"The winning number is: {self.winning_number}.")
print(f"I'm thinking of a number between {self.MIN} & {self.MAX}.")
def guess_and_check(self) -> None:
"""Compares the player's guess to the winning number."""
# Checks if the player's guess is correct.
if self.player.get_guess() == self.winning_number:
print(f'{self.player} Won!')
self.player.add_win()
else:
print(f'{self.player} Lost!')
def play_loop(self) -> None:
"""Determines whether the player wants to play again."""
while True:
print()
self.generate_random_number()
self.guess_and_check()
while True:
play_again = input("Would you like to play again? (y|n): ")
# Checks if the player wants to play again & handles invalid inputs.
if play_again == "y":
break
elif play_again == "n":
print()
# Checks the player's win count and determines if 'time' or 'times' should be used.
if self.player.wins == 1:
print(f'{self.player} won one time!')
else:
print(f'{self.player} won {self.player.wins} times!')
return
else:
print("Please enter either 'y' or 'n'.")
def play_random_number_guesser() -> None:
print("Welcome To Random Number Guesser!")
print("=================================")
game = RandomNumberGuesser()
game.create_player()
game.greet_player()
game.play_loop()
if __name__ == "__main__":
play_random_number_guesser()
Welcome To Random Number Guesser!
=================================
What is your name?: me
Welcome, me!
I'm thinking of a number between 1 & 10.
Can you guess what it is?: 2
me Lost!
Would you like to play again? (y|n): y
I'm thinking of a number between 1 & 10.
Can you guess what it is?: 3
me Lost!
Would you like to play again? (y|n): n
me won 0 times!