OOP Hangman Game

Question

My daughter and I are trying to teach ourselves Python and OOP. We wrote a hangman game, and we'd really appreciate some feedback. Are we heading in the right direction? How could we improve it? Anything that makes a Python programmer go "ouch"? Anything that makes an OOP expert cringe? All sorts of questions occur to us. For example, should we be instantiating classes when there is only one instance? Feel free to be brutal! We've posted it as a repl here: https://replit.com/@scripta/Hangman#main.py

Changed the invoking functionality into a class "Game" as per ASM's suggestion.

A huge thanks for the time and effort that went into commenting on the code. With apologies for having initially changed the code in this initial post (see above), here is a new post with most of the suggestions acted upon: OOP Hangman Game - amended code

import random

class Word:

    def __init__(self):
        words = ("foxglove", "captain", "oxygen", "microwave", "rhubarb")
        self._word = random.choice(words)
        self.letters_in_word=set(self._word)

    @property
    def word(self):
        return self._word

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        progress_string= ""
        for char in self.word:
            if guesses.guessed(char):
                progress_string= progress_string+" "+char+" "
            else:
                progress_string= progress_string +" _ "
        return(progress_string)

    def guessed(self, guesses):
        letters_guessed = self.letters_in_word.intersection(guesses.guesses_made)
        if letters_guessed == self.letters_in_word:
            return True
        else:
            return False


class Guesses:

    def __init__(self):
        # guesses holds all guesses made (wrong or right)
        self.guesses_made = set()

    def guessed(self,char):
        if char in self.guesses_made:
            return True
        else:
            return False

    def record(self,guess,word):
        # All valid guesses (wrong or right) are added to the guesses set
        self.guesses_made.add(guess)
        if not guess in word.letters_in_word:
            Gallows.record_bad_guess()

    def made(self):
        # to sort the set of guesses made we need to cast it to a list
        guesses_list= list(self.guesses_made)
        guesses_list.sort()
        guesses_string=""
        #comma separate the guesses
        for char in guesses_list:
            guesses_string= guesses_string+char+","
        return guesses_string

class Gallows:

    _bad_guesses=0

    gallows_images = ("      \n      \n        \n         \n        \n__________",
                      "      \n |    \n |      \n |       \n |      \n_|________",
                      " _____\n |    \n |      \n |       \n |      \n_|________",
                      " _____\n |/   \n |      \n |       \n |      \n_|________",
                      " _____\n |/  |\n |      \n |       \n |      \n_|________",
                      " _____\n |/  |\n |   O  \n |       \n |      \n_|________",
                      " _____\n |/  |\n |   O  \n |   |   \n |      \n_|________",
                      " _____\n |/  |\n |   O  \n |  /|   \n |      \n_|________",
                      " _____\n |/  |\n |   O  \n |  /|\\ \n |      \n_|________",
                      " _____\n |/  |\n |   O  \n |  /|\\ \n |  /   \n_|________",
                      " _____\n |/  |\n |   O  \n |  /|\\ \n |  / \\\n_|________",)

    @classmethod
    def record_bad_guess(cls):
        cls._bad_guesses +=1

    @classmethod
    def hanged(cls):
        if cls._bad_guesses >= len(cls.gallows_images)-1:
            return True
        else:
            return False

    @classmethod
    def draw(cls):
        return cls.gallows_images[cls._bad_guesses]

class Game:

    def get_letter(self, guesses):
        while True:
            char= input("\nPlease enter your guess: ")
            if len(char)<1:
                print ("You didn't choose a letter!")
            elif len(char)!=1:
                print ("One letter at a time!")
            elif not char.isalpha():
                print ("Alphabetic characters only!")
            elif guesses.guessed (char):
                print ("You already guessed that letter")
            else:
                break       
        return (char)

    def display_progress(self, target, gallows, guesses):
        print("\n",target.progress(guesses)) 
        print(gallows.draw())
        print("\nUsed: \n",guesses.made())

    def play(self):
        target= Word()
        guesses= Guesses()
        gallows= Gallows()
        while True:
            guess= self.get_letter(guesses)
            guesses.record(guess,target)
            self.display_progress(target, gallows, guesses)
            if target.guessed(guesses):
                print("\nYou win, well done")
                break
            if gallows.hanged():
                print ("\nI win. The word was:",target.word)
                break

if __name__ == "__main__":
    game= Game()
    game.play()      

Answer 1

From the first looks, quite nice. I have mostly just a few smaller concerns:

• Consistency matters for readers, so should be either x = y, x= y or x =y (or any variant really). Using an autoformatter is of course easier than doing that manually, but otherwise I'd generally go for the first variant.
• For words: if you have a sequence, use a list, not a tuple. It's clearer and fits the purpose better, e.g. if you were to add new elements to the list of words it only works with list, because tuple's are immutable.
• Negating in can be done with not in, which reads more naturally.
• The pattern if x == y: return True else: return False is not making use of the fact that x == y already returns a boolean value - it can simply be replaced by the much shorter return x == y.
• Some members have an underscore, some don't - unless you strife for a very hard border between private and public properties I'd pick one and stick with it, if you go with the underscores, then just provide @property accessors for them if necessary.
• Each of the images could probably be easier to read by using multi-line strings ("""these ones""").
• If you want to get into list comprehensions, made's way of formatting can be a little bit simpler via str's join method, e.g. ",".join(self.guessed_made).
• sorted can be simpler than list followed by sort, e.g. sorted(self.guesses_made) does what you think it does.

Structurally, the next thing I saw was that Guesses uses the Gallows class, not an object, while in play_hangman.py it's being constructed as an object and also used like one. I'd pick one and stick with it. Since it's intended to be more OOP, right, I'd go with dropping the class methods and make it an actual object, that seems cleaner and at least in theory would allow for better testing anyway.

Wrt. an overall Game class: if you want to do it for learning reasons, sure, but really in Python at least it's fine to have standalone functions too. Of course you can do things like mocking all your dependencies that way, but even just having play_game take target, guesses and gallows as parameters would still allow you to do that. Edit: And I'm not gonna follow that edit since I was already typing this out before.

play_hangman.py:

import hangman


def get_letter(guesses):
    while True:
        char = input("\nPlease enter your guess: ")
        if len(char) < 1:
            print("You didn't choose a letter!")
        elif len(char) != 1:
            print("One letter at a time!")
        elif not char.isalpha():
            print("Alphabetic characters only!")
        elif guesses.guessed(char):
            print("You already guessed that letter")
        else:
            break
    return char


def play_game():
    target = hangman.Word()
    guesses = hangman.Guesses()
    gallows = hangman.Gallows()
    while True:
        guess = get_letter(guesses)
        guesses.record(gallows, guess, target)
        print("\n", target.progress(guesses))
        print(gallows.draw())
        print("\nUsed: \n", guesses.made())
        if target.guessed(guesses):
            print("\nYou win, well done")
            break
        if gallows.hanged():
            print("\nI win. The word was:", target.word)
            break


if __name__ == "__main__":
    play_game()

hangman.py:

import random


class Word:
    def __init__(self):
        words = ["foxglove", "captain", "oxygen", "microwave", "rhubarb"]
        self.word = random.choice(words)
        self.letters_in_word = set(self.word)

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        return "  ".join(char if guesses.guessed(char) else "_" for char in self.word)

    def guessed(self, guesses):
        letters_guessed = self.letters_in_word & guesses.guesses_made
        return letters_guessed == self.letters_in_word


class Guesses:
    def __init__(self):
        # guesses holds all guesses made (wrong or right)
        self.guesses_made = set()

    def guessed(self, char):
        return char in self.guesses_made

    def record(self, gallows, guess, word):
        # All valid guesses (wrong or right) are added to the guesses set
        self.guesses_made.add(guess)
        if guess not in word.letters_in_word:
            gallows.record_bad_guess()

    def made(self):
        return ",".join(sorted(self.guesses_made))


class Gallows:
    gallows_images = (
        "      \n      \n        \n         \n        \n__________",
        "      \n |    \n |      \n |       \n |      \n_|________",
        " _____\n |    \n |      \n |       \n |      \n_|________",
        " _____\n |/   \n |      \n |       \n |      \n_|________",
        " _____\n |/  |\n |      \n |       \n |      \n_|________",
        " _____\n |/  |\n |   O  \n |       \n |      \n_|________",
        " _____\n |/  |\n |   O  \n |   |   \n |      \n_|________",
        " _____\n |/  |\n |   O  \n |  /|   \n |      \n_|________",
        " _____\n |/  |\n |   O  \n |  /|\\ \n |      \n_|________",
        " _____\n |/  |\n |   O  \n |  /|\\ \n |  /   \n_|________",
        " _____\n |/  |\n |   O  \n |  /|\\ \n |  / \\\n_|________",
    )

    def __init__(self):
        self._bad_guesses = 0

    def record_bad_guess(self):
        self._bad_guesses += 1

    def hanged(self):
        return self._bad_guesses >= len(self.gallows_images) - 1

    def draw(self):
        return self.gallows_images[self._bad_guesses]

Answer 2

(Some of these points have already been covered by the answer already posted, but I was already halfway through writing this when it appeared.)

---

This looks like a fun programme, and has got a lot of great stuff in it! This isn't a full review by any means, but here's a few small tips for improving your code:

1. Move the words variable out of the Word.__init__ method.

This variable is an arbitrary constant that isn't altered during execution of the program. You should try to minimise the extent to which such constant values are buried in the middle of your code, so that it is obvious where they are and easy to modify their values should you find yourself editing your code in the future. It's also good practice to give them ALL_CAPS names, which is the convention for constant values that never change. (Your Gallows.gallows_images attribute should be renamed as Gallows.GALLOWS_IMAGES for the same reason.)

Constant values should either go at the top of a module in the global namespace, or at the top of a class in the class namespace. In this case, I'd put it at the top of the class:

import random

class Word:
    WORDS = ("foxglove", "captain", "oxygen", "microwave", "rhubarb")

    def __init__(self):
        self._word = random.choice(self.WORDS)
        self.letters_in_word = set(self._word)

2. Keeping to the PEP8 style guide will make your code more readable for other people.

In particular, there are lots of places in your code where you don't have spaces around operators such as + and =, which I personally find quite hard to read.

3. Using F-strings is a more pythonic way of concatenating strings

In your Word.progress method, you start with an empty string, and add values to it iteratively using the + operator. You could make this more readable by using the += operator to add characters to your string, and by using F-strings to reduce your other uses of the + operator:

class Word:
    # <--- snip --->

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        progress_string = ""
        for char in self.word:
            if guesses.guessed(char):
                progress_string += f" {char} "
            else:
                progress_string += " _ "
        return(progress_string)

You could even use Python's ternary operator to shorten this even further:

class Word:
    # <--- snip --->

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        progress_string = ""
        for char in self.word:
            progress_string += f" {char} " if guesses.guessed(char) else " _ "
        return(progress_string)

Or use the str.join method and a generator expression to make it into a one-liner!

class Word:
    # <--- snip --->

    def progress(self, guesses):
        # create string of underscores and guessed letters to show progress to guessing word
        return "".join((f" {char} " if guesses.guessed(char) else " _ ") for char in self.word)

4. Your methods that return bool values can be simplified

Your Word.guessed method can be simplified like so:

class Word:
    # <--- snip --->

    def guessed(self, guesses):
        return bool(self.letters_in_word.difference(guesses.guesses_made))

Your Guesses.guessed method can be simplified like so:

class Guesses:
    # <--- snip --->

    def guessed(self, char):
        return char in self.guesses_made

And your Gallows.hanged method can also be shortened to one line:

class Gallows:
    # <-- snip -->

    @classmethod
    def hanged(cls):
        return cls._bad_guesses >= (len(cls.gallows_images) - 1)

It's a little unclear to me why your Gallows._bad_guesses attribute is marked as private, by the way. I think it would be fine to have it as a public Gallows.bad_guesses attribute. It's also a bit of a code smell to have a class where every method is a classmethod — for a more comprehensive refactoring of your Gallows class, I'd take a look at @ferada's answer.

I also agree with @ferada that there's no real need for your Game class, as none of its functions keep track of internal state at all. If your code is purely procedural, it's more pythonic to have your functions in the global namespace rather than creating objects for the sake of objects (this is very different to other languages such as Java).

Answer 3

Welcome to Code Review!

If you just started learning, it seems decently good. Here are a few comments in no particular order:

Edit: After posting the answer, I've seen most it not all the points below have already being covered in other answers written while I was writing mine. Still, I will leave it here as it might be useful

• In general, have a read at PEP-8. It is a general style guide widely followed by the community. For example, you are not using consistent spacing:

# No slace on the left of the assignment
progress_string= progress_string+" "+char+" "
# Space on the left
self.guesses_made = set()

• Regarding OOP design, Gallows is poorly designed. It is used as a king of namespace, having all methods being classmethods, it has class properties, but it is also instatiated. It should simply be a class with its properties and its methods.

• Useage of bools:

if condition:
  return True
else:
  return False

Can be changed to:

retun condition

For example, the guessed method in Guesses

• The Guesses#made method can be simplified:

sorted_guesses = list(self.guesses_made).sort()
return ",".joint(sorted_guesses)

• Guesses is just a wrapper around a set, so it probably should not be a class, and just a property in the game object. The Guesses#made method could be a method a Game method. The Guesses#record method should not depend on Gallows (it doesn't really make sense), so it its just a set#add.

• To me, method names should have meaning all by themselves. For example, made does not tell me anything unless I read it as Guesses#made. So it probably should be renamed to something like made_guesses.

• word, guesses and gallows should be Game's properties created upon initialization. They could also be created on a method called init or reset that was called upon initialization. This would allow restarting a game in a more explicit way (currently you could just call play twice)