Object Oriented Hangman in Python Version 2.0

Question

I am looking for as much criticism as possible. Specifically I am trying to institute BEST PRACTICES for OOP and game design. Enjoy!

Github hangman repository

Each Class is in a separate file and imported.

BASIC LETTERS CLASS

from enum import Enum
class Letters(Enum):
 a = ('A', 'A')
 b = ('B', 'B')
 c = ('C', 'C')
 d = ('D', 'D')
 e = ('E', 'E')
 f = ('F', 'F')
 g = ('G', 'G')
 h = ('H', 'H')
 i = ('I', 'I')
 j = ('J', 'J')
 k = ('K', 'K')
 l = ('L', 'L')
 m = ('M', 'M')
 n = ('N', 'N')
 o = ('O', 'O')
 p = ('P', 'P')
 q = ('Q', 'Q')
 r = ('R', 'R')
 s = ('S', 'S')
 t = ('T', 'T')
 u = ('U', 'U')
 v = ('V', 'V')
 w = ('W', 'W')
 x = ('X', 'X')
 y = ('Y', 'Y')
 z = ('Z', 'Z')
 @property
 def unused(self):
 return self.value[1]
 @property
 def dead(self):
 return self.value[0]

GAME LETTERS CLASS

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

UI Class

from GameLetters import GameLetters
import random
class UI(GameLetters):
 def __init__(self):
 self.game_word = self.initialize_game_word()
 self.current_letter = ''
 self.board_letters = []
 self.guessed_letters = []
 self.wrong_guesses = 0
 self.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 get_hangman_drawing(self, failed_attempts):
 
 return self.hangman_drawings[failed_attempts]
 def initialize_game_word(self):
 game_words = []
 with open('words.txt') as file:
 for line in file:
 game_words.append(line.rstrip('\r\n'))
 word = random.choice(game_words)
 return word
 def display_game_word(self):
 
 display_word = ''
 for letter in self.game_word:
 if str(letter) in self.guessed_letters:
 display_word += f"{letter} "
 elif str(letter).upper() in self.board_letters:
 display_word += "_ "
 else:
 print("ERROR: That letter isnt found")
 return display_word
 
 def guess_letter(self):
 while True:
 letter = input("GUESS: ")
 if len(letter) == 1:
 break
 print("Please Only Choose One Letter at a Time")
 letter = letter.strip().lower()
 self.guessed_letters.append(letter)
 self.current_letter = (letter)
 
 def compare_letter(self):
 
 if self.current_letter in self.game_word:
 print("This Letter was found")
 if "_" in self.display_game_word():
 return True
 else:
 print("You Won")
 return False
 
 else:
 print("This letter was most definitely not in the word")
 self.wrong_guesses += 1
 return True
 
 def display_game_state(self):
 self.board_letters.clear()
 self.board_letters = self.populate_board_letters(self.guessed_letters)
 print(f"\n\n\n{self.get_hangman_drawing(self.wrong_guesses)}\n{self.populate_board_letters(self.guessed_letters)}\n{self.display_game_word()}")

Game LOGIC

from UI import UI
class Game:
 def run(self):
 self.game = UI()
 keep_playing = True
 while keep_playing:
 self.game.display_game_state()
 self.game.guess_letter()
 keep_playing = self.game.compare_letter()
 if self.game.wrong_guesses >= len(self.game.hangman_drawings):
 print("Your man has Hanged. Please Try again!")
 break

Execute File

from Game import Game
if __name__ == '__main__':
 Game().run()
```

Answer 1

The entry point ("execute file") as well as the main game loop both look OK. Your Letters class doesn't make any sense to me. That's, by the way, also not how enums usually are used. And I don't understand why you have GameLetters; you're basically only using it to store a method in a different place.

The overall structure of UI is pretty clear. But to call it UI is maybe a bit misleading, since it basically is the entire game.

There's a bunch of smaller improvements you can do here and there. For reading in the list of possible words, you could just do something like

def initialize_game_word(self) -> str:
 with open('words.txt') as file:
 game_words = file.readlines()
 return random.choice(game_words)

In UI.display_game_word() you don't have to convert to str since the letters already are strings, and you don't have to use str.strip() in UI.guess_letter() since you've already asserted that the letter is a single character. But you probably want to make sure that the letter is validated to be a character between a and z - if someone entered a space you would be trying to add an empty string as a guess (since you've stripped the space).

• Infinite Grasp: Super disappointed in not realizing I didnt need to convert to a string, because it is already a string and not an object. Also, I havent really used ENUMS and I am trying to get comfortable with how they can be used. Do you have any suggestions for projects that may implement ENUMS in a manner more standard?
• ades: I don't know how useful it would be to point at a specific project and its usage - I'd suggest that you look into a tutorial or two where they use enums instead.

Answer 2

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('A') - ord('A')
127215

We can use this to really easily build this list:

from string import ascii_uppercase as uppercase
dist = ord('A') - ord('A')
LETTERS = [(letter, chr(ord(letter) + dist)) for letter in uppercase]
LETTERS[25]
('Z', '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 cleared. 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('A') - 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!")