A note on object oriented programs like this. Usually, defining a class with two methods, one of which being __init__
, means that whatever function is in the class could probably just be standalone.
For instance, Game
is not tracking any sort of state. It seems to be a class just for its own sake. If we remove the class:
def game(): ui = UI() keep_playing = True while keep_playing: ui.display_game_state() ui.guess_letter() keep_playing = ui.compare_letter() if ui.wrong_guesses >= len(ui.hangman_drawings): print("Your man has Hanged. Please Try again!") break
There is zero loss in functionality.
Another example is in GameLetters
:
from Letters import Letters class GameLetters(): def __init__(self): pass def populate_board_letters(self, guessed_letters): for letter in Letters: if letter.unused.lower() in guessed_letters: self.board_letters.append(letter.dead) else: self.board_letters.append(letter.unused) return self.board_letters
Again, the __init__
doesn't need to be there, since you pass
. Reading the functions with that lacking __init__
means that you have to know that this class is to be inherited for self.board_letters
to make sense. This sends a signal to me that really populate_board_letters
should be in the UI
class:
# drop the inheritance class UI: ~snip~ def populate_board_letters(self, guessed_letters): for letter in Letters: if letter.unused.lower() in guessed_letters: self.board_letters.append(letter.dead) else: self.board_letters.append(letter.unused) return self.board_letters
Now the attributes are easy to trace and don't "magically" appear in the instance.
Hangman Drawings
This attribute is invariant and could probably just be a class variable:
class UI: hangman_drawings = [ (" _____\n| |\n|\n|\n|\n|\n__"), (" _____\n| |\n| O\n|\n|\n|\n__"), (" _____\n| |\n| O\n| |\n|\n|\n__"), (" _____\n| |\n| O\n| /|\n|\n|\n__"), (" _____\n| |\n| O\n| /|\\\n|\n|\n__"), (" _____\n| |\n| O\n| /|\\\n| /\n|\n__"), (" _____\n| |\n| O\n| /|\\\n| / \\\n|\n__") ]
I've also broken up each drawing into its own line for easier reading, which declutters the program a bit.
Initialized Word
This doesn't belong in the UI class. At the very least, it should be a @staticmethod
since it doesn't use self
:
class UI: ~snip~ @staticmethod def initialize_game_word(): with open('words.txt') as file: # just collect the lines into a list words = file.readlines() # choose the word and strip off whitespace word = random.choice(words).strip() return word
One other thing I might add is the capability to pass in a different file:
@staticmethod def initialize_game_word(file='words.txt'): with open(file) as fh: # just collect the lines into a list words = fh.readlines() # choose the word, strip off whitespace, and set to uppercase word = random.choice(words).strip().upper() return word
Checking guessed letters
There are a few things going on with checking guessed letter, tracking the last guessed letter, and printing the letters that match the game word. To rework this, I'd track display_word
in a list
with a spot that represents a letter in the game word:
class UI: def __init__(self): ~snip~ self.display_word = ['_' for _ in self.game_word]
The guess_letter
function can then be modified accordingly:
class UI: ~snip~ def guess_letter(self): while True: letter = input("Guess a letter: ").strip().upper() if len(letter) != 1: print("One guess at a time, please") elif letter in self.guessed_letters: print("You've already guessed that letter!") print(f"Try one not in {self.guessed_letters}") else: break self.guessed_letters.add(letter) if letter not in self.game_word: print("That letter is not in the word") self.wrong_guesses += 1 return print("That letter was found! Good guess!") for i, char in enumerate(self.game_word): if letter == char: self.display_word[i] = letter # and to refactor the display function def display_game_word(self): # much easier print(self.display_word)
Note how this rids us of the need to have a compare_letter
function, which really is a shorthand of letter in game_word
at its core. I've also forced letter
to be uppercase, since that's how your display letters are formatted.
Since display_game_word
doesn't do much at this point, we can get rid of that function entirely.
Showing used letters
Because the only use for Letters
is to iterate over it and pick either unused
or dead
, I'd say an Enum
is not the right choice for a data structure. I'd lean towards a list
here. A trick to build it quickly is that there is a constant distance between your uppercase and blocked letters:
ord('π°') - ord('A') 127215
We can use this to really easily build this list:
from string import ascii_uppercase as uppercase dist = ord('π°') - ord('A') LETTERS = [(letter, chr(ord(letter) + dist)) for letter in uppercase] LETTERS[25] ('Z', 'π
')
And now, to use your function:
class UI: ~snip~ def populate_board_letters(self, guessed_letters): for letter, dead in LETTERS: if letter in guessed_letters: self.board_letters.append(dead) else: self.board_letters.append(letter) return self.board_letters
Looking closer, though, I don't really love that self.board_letters
gets clear
ed. Why not just rebuild it when you call it? Set it as a @property
:
class UI: ~snip~ @property def board_letters(self): return [ dead if letter in self.guessed_letters else letter for letter, dead in LETTERS ]
Now, this highlights that display_game_state
would be most convenient to just print(UI)
. So let's use the __str__
method:
class UI: ~snip~ def __str__(self): return ( f"\n\n\n{self.hangman_drawings[self.wrong_guesses]}" f"\n{self.board_letters}" f"\n{self.display_word}" ) # use like this ui = UI() print(ui)
Notice that now we are directly indexing hangman_drawings
, referencing board_letters
, and also referencing display_word
. We don't need to hide these behind other functions anymore.
Combining Changes
Now, since we've cut down on how many classes are in this program, I think almost all of it, if not all of it, can effectively be in one file. Here's what we have:
from string import ascii_uppercase as uppercase import random dist = ord('π°') - ord('A') LETTERS = [(letter, chr(ord(letter) + dist)) for letter in uppercase] class UI: hangman_drawings = [ (" _____\n| |\n|\n|\n|\n|\n__"), (" _____\n| |\n| O\n|\n|\n|\n__"), (" _____\n| |\n| O\n| |\n|\n|\n__"), (" _____\n| |\n| O\n| /|\n|\n|\n__"), (" _____\n| |\n| O\n| /|\\\n|\n|\n__"), (" _____\n| |\n| O\n| /|\\\n| /\n|\n__"), (" _____\n| |\n| O\n| /|\\\n| / \\\n|\n__") ] def __init__(self, words_file='words.txt'): self.game_word = self.initialize_game_word(words_file) self.display_word = ['_' for _ in self.game_word] self.guessed_letters = set() self.wrong_guesses = 0 def __str__(self): return ( f"\n\n\n{self.hangman_drawings[self.wrong_guesses]}" f"\n{self.board_letters}" f"\n{self.display_word}" ) @property def board_letters(self): return [ dead if letter in self.guessed_letters else letter for letter, dead in LETTERS ] @staticmethod def initialize_game_word(file='words.txt'): with open(file) as fh: # just collect the lines into a list words = fh.readlines() # choose the word, strip off whitespace, and set to uppercase word = random.choice(words).strip().upper() return word def guess_letter(self): while True: letter = input("Guess a letter: ").strip().upper() if len(letter) != 1: print("One guess at a time, please") elif letter in self.guessed_letters: print("You've already guessed that letter!") print(f"Try one not in {self.guessed_letters}") else: break self.guessed_letters.add(letter) if letter not in self.game_word: print("That letter is not in the word") self.wrong_guesses += 1 return print("That letter was found! Good guess!") for i, char in enumerate(self.game_word): if letter == char: self.display_word[i] = letter def game(): ui = UI() while True: print(ui) ui.guess_letter() if ui.wrong_guesses >= len(ui.hangman_drawings): print("Your man has Hanged. Please Try again!") break elif ui.display_word == list(ui.game_word): print("You won!") break
A few last minute tweaks
I'd rename the game
function to main
, the UI
class should be called Game
. I'd also add in a lost
function to check if the player has hit the max number of guesses and a won
function to see if the player guessed the word:
class Game: ~snip~ def lost(self): return self.wrong_guesses >= len(self.hangman_drawings) def won(self): # all letters are filled in properly return self.display_word == list(self.game_word) def main(): game = Game() while not (game.won() or game.lost()): # print the game board directly print(game) game.guess_letter() if game.won(): print("You guessed the word!") else: print("Your man was hanged! Better luck next time!")