I have commented out two attributes (in random_number_guesser.py) which I believe are unnecessary.
Am I correct in assuming that, because the code runs correctly without their implementation, they should not be included in my program?
player.py
class Player:
def __init__(self) -> None:
"""Initializes the player object's attributes."""
self.wins = 0
def create_username(self) -> None:
"""Prompts the player to enter their username."""
attempts = 1
MAX_ATTEMPTS = 3
while attempts <= MAX_ATTEMPTS:
self.username = input("What is your name?: ")
# Checks if the username contains only whitespace.
if not self.username.strip():
if attempts == MAX_ATTEMPTS:
self.username = "Player_1"
else:
print()
print("Your username must contain at least one character.")
else:
self.username = self.username.strip()
break
attempts += 1
def __str__(self) -> str:
"""Returns the player's username as a string."""
return self.username
def get_guess(self) -> int:
"""Returns the player's guess as an integer."""
while True:
try:
self.guess = int(input("Can you guess what it is?: "))
return self.guess
except ValueError:
print()
print("Please enter a whole number.")
def add_win(self) -> None:
"""Adds a win to the player's win count."""
self.wins += 1
random_number_guesser.py
from player import Player
import random
class RandomNumberGuesser:
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."""
MIN = 1
MAX = 10
self.winning_number = random.randint(MIN, MAX)
print()
#DEBUG - Displays the winning number:
#print(f"The winning number is: {self.winning_number}.")
print("I'm thinking of a number between 1 & 10.")
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()
print(f"{self.player} Won!")
self.player.add_win()
else:
print()
print(f"{self.player} Lost!")
def play_again(self) -> None:
"""Determines whether or not the player wants to play again."""
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":
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 self.player.wins == 1:
print(f"{self.player} won {self.player.wins} time!")
else:
print(f"{self.player} won {self.player.wins} times!")
break
else:
print()
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.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\$\begingroup\$ Welcome to Code Review! To help reviewers give you better answers, we need to know what the code is intended to achieve. Please add sufficient context to your question to describe the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. Also, edit the title to simply summarise the task, rather than your concerns about the code. \$\endgroup\$Toby Speight– Toby Speight2023年11月05日 13:18:27 +00:00Commented Nov 5, 2023 at 13:18
-
\$\begingroup\$ Are you the author and/or maintainer of the code? \$\endgroup\$Sᴀᴍ Onᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2023年11月05日 14:45:39 +00:00Commented Nov 5, 2023 at 14:45
-
1\$\begingroup\$ Yes, I am the author of the code. \$\endgroup\$dylanbobbyromero1– dylanbobbyromero12023年11月05日 15:24:56 +00:00Commented Nov 5, 2023 at 15:24
-
\$\begingroup\$ That's... debatable. The code started off as yours, but this version is mostly mine (minus the commented attributes). \$\endgroup\$Reinderien– Reinderien2023年11月05日 20:55:15 +00:00Commented Nov 5, 2023 at 20:55
-
1\$\begingroup\$ It's totally fine - I only brought it up because Sam asked. I won't make the judgement call as to whether it's off-topic (I'll leave that to the mods), but generally, asking how a certain thing works when you haven't written that thing is not supported on CodeReview. \$\endgroup\$Reinderien– Reinderien2023年11月05日 21:56:30 +00:00Commented Nov 5, 2023 at 21:56
2 Answers 2
Am I correct [that] ... they are unnecessary to include in my program?
Good question.
Incorrect. You should continue to include them.
class RandomNumberGuesser:
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: ...
self.player = Player() ...
def generate_random_number(self) -> None:
...
self.winning_number = random.randint(MIN, MAX)
Now, I see where you're coming from, it is totally sensible that you might conclude "bah, useless!" and rip it out. The machine doesn't care. But people do.
programs must be written for people to read, and only incidentally for machines to execute.
-- SICP
In many OO
languages an object constructor is responsible for
declaring / allocating storage for all attributes
which the object might use over its lifetime.
Python is a dynamic language where strictly speaking
this is not necessary -- attributes can be tacked onto
an object, and even destroyed, any time the object is accessible.
But declarations are helpful to humans.
Good manners dictates that we conventionally introduce
every attribute we will ever use, right up front in the constructor.
If we do not yet have some sensible value we just assign None
and move on.
This is a kindness to the Gentle Reader.
Seeing each attribute reminds us to perhaps describe it
in the __init__
constructor """docstring""",
or to write a # comment
for it.
Each class is responsible for maintaining its
class invariants
over the attributes, and that responsibility starts
the moment the constructor returns.
They can be weak or strong invariants.
The stronger the guarantees, usually the easier
it is for a maintenance engineer to reason about object behavior.
So yes, please retain that "catalog" of attributes, even if we're not yet ready to fill them in at object init time.
Here is a possible refactor you might consider:
MIN = 1
MAX = 10
def __init__(self) -> None:
"""Initializes the game object's attributes."""
self.player: Player = self._create_player()
self.winning_number: int = random.randint(self.MIN, self.MAX)
def _create_player(self) -> Player:
"""Creates a new player object."""
player = Player()
player.create_username()
return player
Notice that we're making a stronger guarantee.
Can self.player
be "None
or a Player
" ?
No, now it can only be a Player
.
And similarly we promise to always have an int
.
Now a maintenance engineer has one less case to consider.
No need to worry about potential None
,
as the class promises that that won't happen.
And we can use
mypy
to verify it stays that way, even after weeks of code edits.
-
8\$\begingroup\$ All correct. It's worth mentioning that this isn't just something you made up; open-source tools like pylint throw W0201 when encountering non-
__init__
members. \$\endgroup\$Reinderien– Reinderien2023年11月04日 19:28:08 +00:00Commented Nov 4, 2023 at 19:28 -
\$\begingroup\$ Thanks for the clarification! \$\endgroup\$dylanbobbyromero1– dylanbobbyromero12023年11月04日 23:13:56 +00:00Commented Nov 4, 2023 at 23:13
-
2\$\begingroup\$ Better yet, why should
RandomNumberGuesser
know how to create a player? Have__init__
take an instance ofPlayer
as a required argument instead. \$\endgroup\$chepner– chepner2023年11月05日 21:37:24 +00:00Commented Nov 5, 2023 at 21:37 -
1\$\begingroup\$ In modern CPython, the "machine" does care. The key-sharing
dict
optimization used by CPython to reduce per-instance memory usage relies on the attributes being unconditionally initialized, in the same order, for all instances (which is not possible to guarantee unless__init__
is uniformly initializing them all). So it's not just for humans, it also makes any class with many instances use substantially less memory per-instance. \$\endgroup\$ShadowRanger– ShadowRanger2023年11月05日 21:45:55 +00:00Commented Nov 5, 2023 at 21:45 -
\$\begingroup\$ @ShadowRanger, wow, thank you! I get the "key sharing" notion. But, benchmarks? Please! I imagine someone has gone to the trouble of showing how memory footprint is dramatically lower if a class plays by the rules? \$\endgroup\$J_H– J_H2023年11月06日 03:23:20 +00:00Commented Nov 6, 2023 at 3:23
Unnecessary to the execution of the specific task the code performs is not the same as unnecessary to the proper usage of the code nor is it the same as irrelevant to good maintenance of the code, nor the same as irrelevant to ease in evolving the code.
Code has context and a purpose that extends beyond the algorithm and its output.
Sometimes it doesn’t extend much beyond that, because it’s a onetime effort to get a specific result, that is itself just being used for a one time effort.
Sometimes it is part of an evolving and essential process.
Whether or not attributes, comments and names are important are a function of context and purpose.
Given the code and names in it, this is probably either schoolwork or hobby exploration. If hobby exploration, then it’s up to you and your goals as to what is unnecessary and what is not. If schoolwork, it depends on grading and how you value that grading.
TL;DR: If this is part of a long term collaboration such as a company project or scientific research, it adds useful information for your fellows.
-
\$\begingroup\$ Thank you for the response! I now realize that we should write code that is functional and easy to follow. I would like to clarify that this is not school work. I do not use Chat GPT to generate any code, nor do I copy straight from tutorials. I feel that doing so would defeat the purpose of trying to actually learn the language. \$\endgroup\$dylanbobbyromero1– dylanbobbyromero12023年11月05日 15:48:54 +00:00Commented Nov 5, 2023 at 15:48
-
1\$\begingroup\$ @dylanbobbyromero1: no problem with hobby exploration, my point is that with a hobby exploration, you get to set the goals of your project. It’s like doing a code kata vs code golf. \$\endgroup\$jmoreno– jmoreno2023年11月05日 19:25:25 +00:00Commented Nov 5, 2023 at 19:25