0
\$\begingroup\$

I am just posting this in hopes of someone pointing out any ways I could improve my hangman game.

I know there are properly a lot of silly issues and unnecessary parts to this, but it works and now I am lost at how to shrink the code down or how to tidy it up a little bit.

from graphics import graphics
import random

class Hangman:


    # Word lists
    options = {"movies": {0 : "titanic",
                          1 : "seven",
                          2 : "deadpool"}}

    playing = False
    guessed_letters = []
    word = ""
    show_letters = None
    previous_words = []
    attempts = 7
    mode = None

    def setup():
        while True:
            user_input = input("Please select the game mode you would like to play?").lower()
            Hangman.mode = user_input
            # Checks to see if the mode the player selected exists
            if user_input in Hangman.options:
                # Picks a random word from the selected mode list
                Hangman.word = random.choice(Hangman.options[Hangman.mode])
                # Changes the show letter variable to _ times letters in the 
                # chosen word
                Hangman.show_letters = list("_" * len(Hangman.word))
                # Starts the game loop
                Hangman.playing = True
                Hangman.game_loop()
                break
            # If the selected mode does not exist
            else:
                print("[{}] mode does not exist! Try a different mode".format(user_input))

    # This section is to check if the player has any attempts left
    def check_attempts():
        # If the player has more than 0 attempts
        if Hangman.attempts > 0:
            # Shows the player graphics of the hangman
            print(graphics[Hangman.attempts])

        # If the player has used all of there attempts
        else:
            print(graphics[Hangman.attempts])
            print("\n\nYou lost!\n")
            print("The word was: {}".format(Hangman.word))
            Hangman.playing = False
        # Remove 1 attempt after each incorrect guess
        Hangman.attempts -= 1

    # Check if the player guessed the correct word
    def check_winner():
        while True:
            # If show_letters does not contain any _ you win
            if "_" not in Hangman.show_letters:
                print("\n\Congratulations. You have won!")
                # Ask the player if they would like to play again
                user_input = input("Would you like to play again? [yes/no]").lower()

                if user_input == "yes":
                    Hangman.reset()
                    break
                elif user_input == "no":
                    print("Thank you for playing!")
                    Hangman.playing = False
                    Hangman.setting_up = False
                    break
                # Check for a valid answer
                else:
                    print("That was an invalid answer!")

            # If they haven't won yet break this loop
            else:
                break

    # Inform the player they have already tried the letter they just typed
    # Show the player the letters they have already guessed
    def already_guessed():
        print(" ".join(Hangman.guessed_letters))

    def game_loop():

        # Used to show the selected word. For testing use only
        print(Hangman.word)

        # Start the game loop if playing = True
        while Hangman.playing == True:
            # Winner check
            Hangman.check_winner()
            # Show the word as _*letters in selected word
            # Or h_ll_o if the player has got some letters correct
            print("\nWord:")
            print("".join(Hangman.show_letters), "\n")
            # Get the players guess
            guess = input("Guess a letter: ").lower()

            # If the player types the whole selected word they win
            if guess == Hangman.word:
                Hangman.show_letters = guess
                Hangman.check_winner()

            # Check if the players guess is a letter and only 1 letter not more
            if guess.isalpha() and len(guess) < 2:
                # Let the player know they has guessed that letter already
                if guess in Hangman.guessed_letters:
                    Hangman.already_guessed()
                # Let the player know their guess was incorrect
                elif guess not in Hangman.word:
                    print("\n{} is incorrect!".format(guess))
                    Hangman.check_attempts()

                # Update the shown letters if the player guesses a correct letter
                else:
                    # Check if the guessed letter is in word
                    # The position of the guessed letter in word
                    # The letter that has been guessed
                    for position, letter in enumerate(Hangman.word):
                        # If it is in word change the shown letters to show
                        # The new letter in the correct position
                        if letter == guess:
                            Hangman.show_letters[position] = letter
                    print("\n{} is correct!".format(guess))

                # If the guessed letter isn't in guessed letters already add 
                # it
                if guess not in Hangman.guessed_letters: 
                    Hangman.guessed_letters.append(guess)
            else:
                print("You can only guess letter's and 1 at a time!")

    # Start the entire program
    Hangman.setup()

I know it is a lot of code but I wasn't sure how else to show it all and get help. this is my first game I have finished making. It all works but I have remove some bits to make it shorter to post and it is indented 4 spaces in idle. I haven't used any tabs because I have read they can cause issues. But i have used the idle indent button a few times

\$\endgroup\$
  • \$\begingroup\$ Could you please fix your indentation to make the code executable? Thanks. \$\endgroup\$ – alecxe Aug 30 '17 at 18:21
  • \$\begingroup\$ @alecxe i believe i have fixed the issue. Thank you \$\endgroup\$ – mcloovin123 Aug 30 '17 at 18:36
2
\$\begingroup\$

Well, you have a misunderstanding of what is a Python class. You should take a look at this (detailed) documentation.

First put all imports at the top of the file and define your class as follow (it's a draft):

# hangman.py
import random


class Hangman (object):
    """
    The hangman game.
    """
    def __init__(self, word):
        """
        Initialize the game with the word to guess.

        :type  word: str
        :param word: the word to guess.
        """
        self.word = word

This class is very simple. It has a docstring and a constructor (initialization method).

The initialization method take a single parameter word which is the word to guess. The documentation string describe the function and its parameter. We use the Sphinx syntax here, but there are others… This method initialize the self.word instance variable. Of course there are other things to initialize, we will see that later.

To run this program, a good practice is to define a main function:

def main():
    words = ["titanic", "seven", "deadpool"]
    hangman = Hangman(random.choice(words))
    print(hangman.word)

This function is called that way:

if __name__ == '__main__':
    main()

This allows a developer to use hangman as a library, and not only as a runnable script. For instance:

import hangman
hangman.main()

Then, you can go further…

We can define the game loop. We have a number of attempts, for each attempt we do:

  • show the grid of letters
  • ask a letter to the user and place the letter in the grid if it fit

If the user wins, the loop breaks:

def main():
    words = ["titanic", "seven", "deadpool"]
    attempts = 7

    hangman = Hangman(random.choice(words))
    for attempt in range(1, attempts + 1):
        hangman.show_grid()
        result = hangman.ask_letter()
        if result == "win":
            break
        print("{0} attempts remain".format(attempt))
    print("The word is " + hangman.word)
    print("Goodbye.")

To implement this methods, you need to store the already choose letters. You do that is the constructor:

    def __init__(self, word):
        self.word = word
        self.letters = ""

Then you can define show_grid() method:

    @property
    def grid(self):
        grid = ""
        for l in self.word:
            if l in self.letters:
                grid += l
            else:
                grid += "_"
        return grid

    def show_grid(self):
        print()
        print("You choose: " + self.letters)
        print("Grid: " + self.grid)

The property grid will be reused in the next method:

    def ask_letter(self):
        while True:
            letter = input("Guess a letter: ")
            if letter in self.letters:
                print("This letter is already played, sorry")
                continue
            self.letters += letter
            if "_" in self.grid:
                return "bad letter"
            else:
                return "win"
\$\endgroup\$
  • \$\begingroup\$ Thank you very much. I kept reading about classes but did fully understand them. I will try to learn more about them. Then implement it into my game. \$\endgroup\$ – mcloovin123 Aug 30 '17 at 20:52
  • \$\begingroup\$ Thank you. I have tried out what you said here. It works well, ill just edit it to be more specific and have a mode. Thank you so much. I will keep readying about the classes \$\endgroup\$ – mcloovin123 Aug 30 '17 at 22:13
  • \$\begingroup\$ Do you need more explanation? I suggest you to upvote and accept my answer. \$\endgroup\$ – Laurent LAPORTE Sep 4 '17 at 21:47
0
\$\begingroup\$

Use classes correctly

The very first problem is that you aren't using OOP in Python correctly - try reading this tutorial.

Basically, you want to implement a class that is basically a blueprint - it defines pieces of data that will be present and actions that can be taken. After implementing your class, you want to turn it into an instance of that class. This instance of a class will actually hold the data and perform the actions. Thus your example would end up like this:

game = Hangman()
game.setup()

The rest of my review is going to assume you've changed it all to work that way, but haven't changed anything else. I'll add a self parameter to all of the methods in the class, and I'll set the data members in the __init__ dunder method.

Now, let's talk about how else we can improve the code

Separation of concerns

Note: I'm not a professional game dev, nor do I necessarily know a lot about game dev terminology; this is just how I refer to these things

Right now all of the details of how the game runs (i.e. moving from step to step, prompting the user, etc) and how the game functions (what the word is, if a guess is correct, etc) are all mixed together. For a small game that's fine - it's easy and clear what is going on, but if you ever wanted to get into a more complex situation it could quickly get out of hand. You may want to separate the game into an engine and a driver. How the game runs is the game driver, and how the game functions is the game engine. Separating the logic of the driver and the engine makes for a cleaner game.

Let's look at what the driver needs to do:

  1. Get a word to guess
  2. Get a letter/word guess from the user
  3. Inform the user of the result of their guess
  4. Prompt the user to play again.

The engine, on the other hand, has to do this instead:

  1. Validate a word/letter guess
  2. Determine win/loss

These are related, but not identical concepts.

Engine

Let's look at how I implemented the engine first, as that is much simpler

class GameResults:
    GAME_WON = 1
    ALREADY_GUESSED = 2
    OUT_OF_TRIES = 3
    LETTER_NOT_FOUND = 4
    LETTER_FOUND = 5
    WRONG_WORD = 6

    result_messages = {
        GAME_WON: "You won the game!",
        ALREADY_GUESSED: "You've guessed that letter before!",
        OUT_OF_TRIES: "You ran out of tries - you lose!",
        LETTER_NOT_FOUND: "This letter wasn't in the word - try again!",
        LETTER_FOUND: "You found a letter!",
        WRONG_WORD: "That isn't the right word!"
    }

class Hangman:

    def __init__(self, word, attempts=7):
        self.word = word
        self.guessed_letters = set()
        self.attempts = attempts

    @property
    def display_letters(self):
        result = ""
        for char in self.word:
            if char in self.guessed_letters:
                result += char
            else:
                result += "_"
        return result

    def guess_letter(self, letter):
        if letter in self.guessed_letters:
            return GameResults.ALREADY_GUESSED
        self.guessed_letters.add(letter)
        self.attempts -= 1
        if letter not in self.word:
            if self.attempts == 0:
                return GameResults.OUT_OF_TRIES
            return GameResults.LETTER_NOT_FOUND
        if "_" not in self.display_letters:
            return GameResults.GAME_WON
        return GameResults.LETTER_FOUND

    def guess_word(self, word):
        self.attempts -= 1
        if word == self.word:
            return GameResults.GAME_WON
        return GameResults.WRONG_WORD

The first thing I did was I created an enum1, GameResults, that holds the different "results" of a game action. It also includes what messages you should display for each result2. Pulling the results out into an enum like this does a few things.

  1. It makes it easy for both the game and the engine to know what happened after a game action, and in a clear way
  2. We don't have magic numbers floating around
  3. We've easily pulled out the messages that should be displayed in each situation, instead of hardcoding them. If you ever wanted to repeat the message somewhere, or if you had to change them (i.e. translation) then you only have to change the message in one place.

After that, I streamlined the engine to only include the bare minimum to actually perform the game. It should know what word is being attempted, what guesses have been made so far, and how many attempts are remaining. Beyond that we provide a property that returns the current status of the word. I've chosen to recalculate it every time for simplicity, but you may find that your previous solution is better, or it might be better to do something in-between.

Lastly, I only provide methods to guess a word and to guess a letter. I separated these two functions because they are distinct actions - the way you check if a letter is correct or a word is correct are different, and if we trust our driver then we always know that what was passed in is already a valid word/letter.

Driver

The driver is a bit more complicated, so I won't throw it all at you at once.

The biggest part of the driver is prompting the user to enter some input, performing validation of that input, returning the input if valid, and erroring and repeating if the input is not valid. Instead of repeating that logic for every prompt, I wrote this helper function that works generically.

class DriverErrorCodes:
    SUCCESS = 1
    INVALID_LETTER_LENGTH = 2
    LETTER_NOT_ALPHA = 3
    INVALID_GUESS_TYPE = 4
    LETTER_ALREADY_GUESSED = 5
    UNKNOWN_CATEGORY = 6
    NOT_YES_NO = 7

    err_messages = {
        INVALID_LETTER_LENGTH: "Letter must have exactly 1 character",
        LETTER_NOT_ALPHA: "Letter must be an alphabetic character",
        INVALID_GUESS_TYPE: "The guess type must be either 1 (letter) or 2 (word)",
        LETTER_ALREADY_GUESSED: "You already guessed that letter",
        UNKNOWN_CATEGORY: "The specified category is unknown",
        NOT_YES_NO: "That wasn't 'yes' or 'no'"
    }

def _get_user_input(self, prompt, validator=lambda x: DriverErrorCodes.SUCCESS):
    while True:
        user_input = input(prompt).lower()
        is_valid = validator(user_input)
        if is_valid is not DriverErrorCodes.SUCCESS:
            print(DriverErrorCodes.err_messages[is_valid])
        else:
            return user_input

Similar to the engine we provide an enum with all of the possible errors that could happen for a given user input, and the appropriate error messages for each. We then have a generic function that takes a prompt and a validation function, and gets user input using that prompt until the validation function succeeds. After that, we provide a bunch of functions to get the different user inputs.

def _get_category_prompt(self, header, options, underline="="):
    prompt = "\n".join([header, underline*len(header)])
    options = "\n".join(["\t".format(category) for category in options])
    return "\n".join([prompt, options])

def get_word_to_guess(self):
    prompt = self._get_category_prompt("Categories:", self.options)
    def is_valid_category(category):
        if category not in self.options: return DriverErrorCodes.UNKNOWN_CATEGORY
        return DriverErrorCodes.SUCCESS
    return random.choice(self.options[self._get_user_input(prompt, is_valid_category)])

def get_word_guess(self):
    return self._get_user_input("What word would you like to guess? ")

def get_letter_guess(self):
    def is_valid_letter(letter):
        if len(letter) != 1: return DriverErrorCodes.INVALID_LETTER_LENGTH
        if not letter.isalpha(): return DriverErrorCodes.LETTER_NOT_ALPHA
        if self.game.letter_already_guessed(letter): return DriverErrorCodes.LETTER_ALREADY_GUESSED

        return DriverErrorCodes.SUCCESS

    return self._get_user_input("What letter would you like to guess? ",
                                is_valid_letter)

def get_guess_type(self):
    def is_valid_guess(guess_type):
        if guess_type not in [1, 2]: return DriverErrorCodes.INVALID_GUESS_TYPE
        return DriverErrorCodes.SUCCESS

    return self._get_user_input("Enter 1 to guess a letter, or 2 to guess the word")

def repeat(self):
    def is_yes_no(yn):
        if yn[0].lower() in ["y", "n"]: return DriverErrorCodes.SUCCESS
        return DriverErrorCodes.NOT_YES_NO
    return self._get_user_input("Play again (Y/N)?", is_yes_no)

I won't go into details of how each of those work, but we've specified the details that actually matter for each type of user input (how it is validated and prompted) in the appropriately named function, but left out the details of validation.

At this point, all we need to do is actually run the game!

class HangmanDriver:

    def __init__(self, options=None):
        self.options = options or {"movies": ["titanic", "seven", "deadpool"]}
        self.game = None

    # all those prompting functions above

    def display_status(self):
        print("Current status: {}".format(self.game.display_letters))
        print(graphics[self.game.attempts])

    def loop(self):
        while True:
            word = self.get_word_to_guess()
            self.game = Hangman(word)
            while True:
                self.display_status()
                guess_type = self.get_guess_type()
                if guess_type == 1:
                    letter = self.get_letter_guess()
                    guess_result = self.game.guess_letter(letter)
                elif guess_type == 2:
                    word = self.get_word_guess()
                    guess_result = self.game.guess_word(word)

                print(GameResults.result_messages[guess_result])
                if guess_result = GameResults.GAME_WON:
                    break
            play_again = self.repeat()
            if play_again[0].lower() == "n": break

Here we just loop through the game, and handle all of the prompts and decision making while passing off how the game works to the actual game. At this point we're done! You just need to run your game

if __name__ == '__main__':
    HangmanDriver().loop()

Miscellaneous notes

Here is some misc feedback on general code style and whatnot

  • Your options dictionary is a mapping from categories to what is effectively a list - having the numeric keys adds no values, so just use a list/tuple
  • You don't display the available game modes, which would be super frustrating if you keep trying to enter a new mode but never succeed
  • You repeat the line print(graphics[Hangman.attempts]) in each branch of your if statement - just put it outside.
  • Use if __name__ == '__main__' to run the code so it is safely importable
  • Functions should generally return a result, not just print it. That way if you ever want to do something else with the result, or display it differently, those functions don't need to change
  • You have too many comments - most of them are unnecessary. In general, comments should only be used if they describe why code is written, and only if it isn't clear from the context. Even then, you usually want to move the behavior into a function with a name that describes why that code exists. If they describe how it works, remove it (unless its arcane, in which case rewrite it). If they describe what it does, move them into functions.

  1. Depending on your Python version there may be built-in enums that work better for you, or 3rd-party packages available. YMMV
  2. For true separation of concerns these should be a separate function, but I've left it here for simplicity.
\$\endgroup\$

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.