7
\$\begingroup\$

Please provide your honest feedback. Thanks

Doing Angela Yu's 100 days of code. Have not seen the videos on this section yet. This is just my attempt.

here is the link to run the code. Hangman game

"""Hangman Game"""
import random
import os

#word list
word_list = ["aardvark", "baboon", "camel"]

HANGMANPICS = ['''
  +---+
  |   |
      |
      |
      |
      |
=========''', '''
  +---+
  |   |
  O   |
      |
      |
      |
=========''', '''
  +---+
  |   |
  O   |
  |   |
      |
      |
=========''', '''
  +---+
  |   |
  O   |
 /|   |
      |
      |
=========''', '''
  +---+
  |   |
  O   |
 /|\  |
      |
      |
=========''', '''
  +---+
  |   |
  O   |
 /|\  |
 /    |
      |
=========''', '''
  +---+
  |   |
  O   |
 /|\  |
 / \  |
      |
=========''']
#Randomly choose a word from the word_list and assign it to a variable called chosen_word.
chosen_word = random.choice(word_list)
#creating a list for the guesses
blanks = list("_" * len(chosen_word))

lives = 6 #total no of lives
h = 0 #hangman array index

#Ask the user to guess a letter and assign their answer to a variable called guess. Make guess lowercase.
def ask_input():
  while True:
    guess = input("Guess a letter?  ").lower()
    if guess.isalpha() and len(guess) == 1:
      return guess

# to replace the blanks
def replace_blank(guess):
    """checks if the guess is part of array or not """
    global lives
    global h
    if guess not in chosen_word:
        lives -= 1
        h += 1
    else:
        for i in range(len(chosen_word)): 
            if chosen_word[i] == guess:
                blanks[i] = chosen_word[i]
               
    hangman = HANGMANPICS[h]
    
    os.system('clear')
    
    
    if guess not in chosen_word:
        print(f"You guessed {guess}. That's not in the word. You lose a life.\n")
    print(blanks)
    print(hangman)
    
    return blanks    

#Check if the letter the user guessed (guess) is one of the letters in the chosen_word.
def testing():
    while lives > 0:
        guess = ask_input()
        blanks = replace_blank(guess)
        word_f = ''.join(blanks)
        if word_f == chosen_word:
            print("\n ****You win**** \n ")
            break
        elif lives == 0:
            print("You lose")
      
def main():
    testing()

if __name__ == '__main__':
    main()

```
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

Currently, you have print statements mixed throughout your game logic. I would suggest separating your code into the part that decides what to do with the latest input, and the part that prints a summary of what happened. To do this, you can replace your prints with properties on a game state object. By extracting a GameState object, you can also replace globals with immutable variables, which can make your code easier to think through.

Here is an example GameState object. When updating the game state by a guess, it returns a new GameState that's an update of the old game state.

class GameState:
    def __init__(self, lives, h, blanks, chosen_word):
        self.lives = lives
        self.h = h
        self.blanks = blanks
        self.chosen_word = chosen_word

    def next_state_for_bad_guess(self):
        return GameState(self.lives - 1, self.h + 1, self.blanks.copy(), self.chosen_word)

    def next_state_for_good_guess(self, i, character):
        new_blanks = self.blanks.copy()
        new_blanks[i] = character
        return GameState(self.lives, self.h, new_blanks, self.chosen_word)

    def get_guessed_word(self):
        return ''.join(self.blanks)

    def get_hangman_pic(self):
        return HANGMANPICS[self.h]

    def did_win(self):
        return self.get_guessed_word() == self.chosen_word

    def did_lose(self):
        return self.lives <= 0

    def is_game_over(self):
        return self.did_lose() or self.did_win()

Here's how your code changes if you're using this game state object:

def get_next_state(guess, state):
  if guess not in state.chosen_word:
     return state.next_state_for_bad_guess()
  else:
     for i in range(len(state.chosen_word)): 
         if state.chosen_word[i] == guess:
              return state.next_state_for_good_guess(i, guess)

def print_state(guess, state):
    os.system('clear')
    
    if guess not in state.chosen_word:
        print(f"You guessed {guess}. That's not in the word. You lose a life.\n")
    print(state.blanks)
    print(state.get_hangman_pic())
    if state.did_win():
        print("\n ****You win**** \n ")
    elif state.did_lose():
        print("You lose")
    
def testing():
    chosen_word = random.choice(word_list)
    starting_lives = 6 # total no of lives
    starting_blanks = list("_" * len(chosen_word))
    state = GameState(starting_lives, 0, starting_blanks, chosen_word)
    while not state.is_game_over():
        guess = ask_input()
        state = get_next_state(guess, state)
        print_state(guess, state)

By splitting up the responsibilities like this you can think about fewer things at once. When you're thinking through the game rules, you don't have to think about how to print. When you're thinking about printing, you don't have to think about how the state changes -- only what the current state is. This is especially important as the program grows larger than something that can be entirely fit inside your head.

Regarding immutability, I can see at least two benefits:

  1. Like single responsibility, this reduces the number of things you think about at once. There is exactly one place where the state can change, so if you want to understand state change you look there. Whereas, with multiple global variables, the state can be changed anywhere in the program so to understand state change you have to look at the whole program
  2. You get a lot of powerful options for free once things are immutable. With this version, you could check the consequences of a guess without taking the guess. If you wanted to provide three hints to players, you could implement the hint by calling get_next_state and reporting whether the guess was good or not, without storing the result in state and printing the update. I'm not suggesting you go design you code for features that you don't need; I'm illustrating how separating the state change logic from the state storage logic opens up cool possibilities that we don't have when the state change logic is writing directly to our state variables.
\$\endgroup\$
7
\$\begingroup\$

If a function just calls another function, delete one of them. You don't need both main() and testing(), since the former does nothing other than call the latter.

Global variables are almost never needed. Don't organize programs around the need to modify global variables. Instead, organize your programs around functions that rely only on their local variables (or methods that follow similar principles). If some functions need access to the same information, pass that data back and forth using function arguments and return values.

Don't put logic in top-level code. At the top level of a program, you can perform imports or define constants, functions, or classes. Put all other code inside of functions or methods. If we apply all of the preceding ideas to your current code, we end up with the following structure:

def main():
    lives = 6
    chosen_word = random.choice(WORD_LIST)
    ...
    while lives > 0:
        guess = ask_input()
        blanks, lives = replace_blank(guess, chosen_word, blanks, lives)
        ...

def ask_input():
    ...

def replace_blank(guess, chosen_word, blanks, lives):
    ...
    return (blanks, lives)

Try to design functions so that they perform a single task. This reorganization of the code highlights some awkwardness in the implementation. The main() function initializes lives and blanks but then delegates responsibility for updating them to the replace_blank() function. Meanwhile, that function has various other responsibilities, like printing messages to the user and clearing the terminal.

Missing features. Many hangman games tell the user what letters have already been guessed and I've never seen one that penalizes a player for guessing an already-guessed letter. I mention this missing behavior because it points the way toward a less awkward code design. The first step is to select a data structure to hold prior guesses. A set is a natural fit. In fact, if we know the initial number of lives, the secret word, and the prior guesses, all other information about game state can be derived (remaining lives, whether the game is finished, how the secret word should be displayed, etc).

Try to separate logic/algorithm from interactivity and printing. Code that interacts with users is a hassle to test and debug. Ideally, you want most of your program to operate purely in the data realm: for functions that means receiving data and returning data; for methods it might mean receiving data and then returning data or modifying internal data. However, many programs require interactivity and related printing. Programs like this are often good fits for a design that used a main function to handle interactions and an immutable object that knows hold to hold the current state and then how to compute the next state. Let's start with a sketch of the interactive layer. This is where we perform all printing and collecting of user input. Notice especially that this part of the program contains very little algorithmic logic.

import random
import os

WORD_LIST = [...]

HANGMANPICS = [...]

def main():
    # Setup.
    gs = GameState(random.choice(WORD_LIST))

    # Play until done.
    while not gs.is_finished:
        # Printing.
        os.system('clear')
        print(gs.graphic)
        print('Secret word:', gs.display_word)
        print('Prior guesses:', gs.display_guesses)
        if gs.message:
            print(gs.message)
        # Derive the next state based on user input.
        gs = gs.next_state(ask_input())

    # Wrap up.
    print(gs.outcome)

def ask_input():
    while True:
        guess = input("Guess a letter?  ").lower()
        if guess.isalpha() and len(guess) == 1:
            return guess

if __name__ == '__main__':
    main()

The rest of the code just requires implementing the needed data-retrieval behaviors in GameState. As noted, the initial state will hold an empty set for prior guesses. A message attribute will be used to store information needed by our interactive layer when printing game status.

from dataclasses import dataclass, field

@dataclass(frozen = True)
class GameState:
    word : str
    prior_guesses : set = field(default_factory = set)
    message : str = ''

    INITIAL_LIVES = len(HANGMANPICS) - 1
    BLANK = '_'

Inside the GameState class, we need to implement various read-only properties to get information derived from our core pieces of data (the word and the prior guesses):

class GameState:

    @property
    def lives(self):
        return self.INITIAL_LIVES - sum(
            char not in self.word
            for char in self.prior_guesses
        )

    @property
    def display_word(self):
        return ''.join(
            char if char in self.prior_guesses else self.BLANK
            for char in self.word
        )

    @property
    def display_guesses(self):
        return ' '.join(sorted(self.prior_guesses))

    @property
    def is_finished(self):
        return self.lives <= 0 or self.word == self.display_word

    @property
    def graphic(self):
        return HANGMANPICS[self.INITIAL_LIVES - self.lives]

    @property
    def outcome(self):
        return (
            None if not self.is_finished else
            'You won' if self.display_word == self.word else
            'You lost'
        )

Finally, the class needs a method to generate a new GameState instance based on the current state and the user's guess:

class GameState:

    def next_state(self, guess):
        w = self.word
        pg = self.prior_guesses
        message = (
            f'Already guessed: {guess}' if guess in pg else
            f'Correct: {guess}' if guess in w else
            f'Not in word: {guess}. You lose a life'
        )
        return GameState(w, pg | {guess}, message)

With a design like this, the vast majority of your functionality could be tested and debugged purely by working with the data-oriented GameState class.

\$\endgroup\$
10
  • \$\begingroup\$ Why are you declaring basically all of the GameState methods as properties? This doesn't make much sense to me. \$\endgroup\$
    – ades
    Jan 9 at 14:46
  • \$\begingroup\$ @ades Because the alternatives are worse. Consider lives as an example. (1) Option 1: store it as an attribute. Bad idea, because lives is purely derived from word and prior guesses. If you store lives separately, you have to manage correlated data, which is error prone. (2) Option 2: provide lives via a regular method. That works alright, but it does force the user to type gs.lives() rather than gs.lives. \$\endgroup\$
    – FMc
    Jan 9 at 16:31
  • \$\begingroup\$ I think having is_finished as a property makes sense, but things like outcome and display_word should not be properties. I think you can take two routes - either follow google's style guide and define properties by how trivial the operation is, or try to deduce according to behaviour. And in either of those cases I don't think all those methods should be properties. \$\endgroup\$
    – ades
    Jan 9 at 16:41
  • \$\begingroup\$ @ades "Things like outcome and display_word should not be properties". I would/could be more convinced if you gave a reason. I say that it a non-snarky way. \$\endgroup\$
    – FMc
    Jan 9 at 16:58
  • \$\begingroup\$ Properties for me are for attributes/members. Displaying a word is a process, which is confusing to mix with the operations that you use for attributes. I'd wonder what was going on if I read a call that was GameState.display_word - I could see how GameState.current_word would be a property. That would be the behavioural deduction. If we instead use Google's "classifier", you could also argue that the operation that you perform isn't trivial enough; you have a conditional and you transform type. This one is less clear ("what is trivial"), but I think you get the point. \$\endgroup\$
    – ades
    Jan 9 at 17:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.