2
\$\begingroup\$

I have just written a hangman game in Python for a school project. It reads in a list of words from a text file, and graphics from another text file.

For each word, the program asks keeps asking for the user's guess, and prints out either the updated word or incorrect letters, until either the number of attempts reaches the limit, or the user guesses the word correctly. It allows the user to guess again if a letter has already been guessed. This repeats for the other words in the list (if the user wishes to play).

It works perfectly fine, but I feel that it is unnecessarily long, particularly with the program flow (conditional statements).

def ReadWords(filePath):
    """
    Read in a file containing words and return a list of individual
    words.
    """
    try:
        with open(filePath, "r") as f:
            wordsList = f.read().split(",")
    except IOError as error:
        print("An error occurred when reading the file!")
    else:
        return wordsList


def ReadGraphics(filePath):
    """
    Read in the graphics file and return a two-dimensional list where
    each sublist is the next graphic, containing a list of strings that
    make up that image.
    """
    try:
        with open(filePath, "r") as f:
            fileGraphics = f.readlines()
    except IOError as error:
        print("An error occurred when reading the file!")
    else:
        graphicsData = []
        for line in range(0, len(fileGraphics), 10):
            graphicsData.append(fileGraphics[line : line + 10])
    return graphicsData


def CreateDisplayWord(currentDisplayWord, guessWord, correctLetter):
    newDisplayWord = ""
    for letter in range(len(guessWord)):
        if guessWord[letter] == correctLetter:
            newDisplayWord += correctLetter
        elif currentDisplayWord[letter] != "_":
            newDisplayWord += guessWord[letter]
        else:
            newDisplayWord += "_"

    return newDisplayWord


words = ReadWords("Words.txt")
graphics = ReadGraphics("Graphics.txt")

keepPlaying = "y"
incorrectGuesses = 0

numWord = 1
score = 0

while keepPlaying == "y":
    # Print the number of the word
    print("WORD {} OF {}".format(numWord, len(words)))
    # Select the hidden word from the list
    hiddenWord = words[numWord - 1]

    incorrectGuesses = 0
    # Initialise the display word as blank spaces
    displayWord = "_" * len(hiddenWord)
    # Initialise lists of correctly guessed letters and incorrectly guessed letters
    correctLetters = []; incorrectLetters = []

    while displayWord != hiddenWord and incorrectGuesses < 11:
        userGuess = input("Guess a letter: ")

        # Check if letter has already been guessed
        if userGuess in correctLetters or userGuess in incorrectLetters:
            print("You have already guessed this letter! Please try again!")
        else:
            # Check if guess is in the hidden word
            if userGuess in hiddenWord:
                # Add the guess to the list of correct letters
                correctLetters.append(userGuess)
                # Replace the blank spaces with the correct letter and display
                displayWord = CreateDisplayWord(displayWord, hiddenWord, userGuess)
                # Check if user has guessed the word
                if displayWord == hiddenWord:
                    print("Well done! You guessed the word!")
                    score += 1
                    break
                else:
                    print(displayWord)
            else:
                # Increment the number of incorrect guesses
                incorrectGuesses += 1
                # Add the guess to list of incorrect letters
                incorrectLetters.append(userGuess)
                # Print the corresponding graphic
                for line in graphics[incorrectGuesses - 1]:
                    print(line)
                # Check if user has used all his attempts
                if incorrectGuesses == 11:
                    print("Unlucky, you have used all your attempts! The word was " + str(hiddenWord))
                    break
                # Update the user with which letters they have guessed incorrectly
                print("Incorrect Letters: " + str(incorrectLetters))

    # Check if all the words have been played
    if numWord == 4:
        userName = input("Enter your name: ")
        print("{} you scored {} out of {}".format(userName, score, len(words)))
        break
    else:
        # Ask user if they wish to play again
        keepPlaying = input("Do you want to play the next word? (y/n): ")
        # Quit is user wants to
        if keepPlaying != "y":
            break
        # Move on to the next word
        numWord += 1

I would appreciate if someone could give some feedback on this issue.

\$\endgroup\$
1
2
\$\begingroup\$

By no means is this a complete list of suggestions for your code, but two that strike me are the following:

Validating input

keepPlaying = input("Do you want to play the next word? (y/n): ")

This is a dangerous line because you do not validate your input. Many of your other IO lines do a great job of validating the user's input, but this one does not. Note that, in general, you should avoid using strings unless absolutely necessary, but keepPlaying is, for all intents and purposes, a boolean value, because it is used only in the context keepPlaying == "y". Thus you should validate this input once on input, and from then on, keepPlaying should be a boolean value. This will protect you from unexpected behaviour (what if the user accidentally inputs yes, or no).

On a similar note, you may want to validate the input from this line as well:

userGuess = input("Guess a letter: ")

not only by checking if the input was already guessed, but also by checking if the user input was a single character and not a combination of characters. You assume the input is a letter everywhere else in your code, so insist that it is one here. This could be as simple as:

if len(userGuess) == 1:
    # continue normally
else:
    # report error

Don't misinterpret "avoid using strings unnecessarily" as "don't use strings". We need strings in programming. However, the problem with your use in keepPlaying == "y" is that you need to repeat this "y" everywhere you check the value of keepPlaying. What if you make a typo? Or decide you want the full word "yes". Now you'll have to change that in every instance where you check the value of keepPlaying. It is simply easier to maintain if you check keepPlaying when it is input, and covert it to a boolean immediately.

Main Function

I would also suggest including all your code after the function definitions in the following code block:

if __name__ == "__main__":
    # your code here...

Here is the long answer for why you might want to, but the short answer is that this allows you to import your module functions without running the hangman game, which allows you to use the functions outside of this particular file. This is not required and I certainly have files that do not do this, but it also adds some clarity to your code by separating function definitions from your main code, and I think it is a worthwhile inclusion in your code.

\$\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.