4
\$\begingroup\$

As it is now the end of my semester for my programming class, we had a final test were we were supposed to make a short hangman game. I have since added some small improvements, such as showing the word when you lose and dynamic difficulty.

import random


def word_loader():
    """Loads a random word from the word list."""
    # Opens the word list document and puts into a list format
    with open("words.txt") as file:
        lines = file.readlines()
    randNum = random.randint(1, len(lines) - 1) # Selects a random number from 1 to the number of words
    return lines[randNum][:-1] # Removes the trailing new line


def word_updater(guessedLetters, word):
    """Takes an input and makes sure its clean."""
    global guessCount
    while True:
        userData = input("What letter would you like to guess? ")
        userData.lower() # Makes all letters in the input lower case
        # Checks if a letter has been guessed before
        if userData in guessedLetters:
            print("You have guessed that letter before, please use another letter.")
        # Checks if there is more than one letter
        elif len(userData) != 1:
            print("Please enter only one letter.")
        # Check if it contains only letters
        elif not userData.isalpha():
            print("Please only enter letters.")
        else:
            guessedLetters.append(userData)
            if userData not in word:
                guessCount = guessCount - 1
            return guessedLetters


def info_adder(wordData, guessedLetters):
    """Fills wordData with the revealed letters."""
    for letter in enumerate(word):
        # If the letter in the word is in guessedLetters, set that locatin in wordData with that letter
        if letter[1] in guessedLetters:
            wordData[letter[0]] = letter[1]
    return wordData


def word_printer(word, guessCount, guessedLetters):
    """Prints out unerscores for unguessed letters and the letter for correctly guessed letters."""
    wordData = [] # An empty list to store what letters have been guessed
    # Fills the empty list with underscores
    for _ in enumerate(word):
        wordData.append("_")
    wordData = info_adder(wordData, guessedLetters)
    print(" ".join(wordData)) # Prints the list with each character seperated by a space
    print(f"You have {guessCount} wrong guesses left")


def win_checker(word, guessedLetters):
    """Checks if game has been won."""
    correctLetters = 0
    for letter in enumerate(word):
        # If a letter in word is in guessedLetters, that means it has been correctly guessed, so incremeant the counter
        if letter[1] in guessedLetters:
            correctLetters += 1
    if correctLetters == len(word):
        return True
    return False


guessReset = 8 # What the guessCount is reset to
while True:
    # Initialize's the varibles
    guessCount = guessReset
    guessedLetters = []
    word = word_loader()
    word_printer(word, guessCount, guessedLetters)
    while guessCount != 0 and not win_checker(word, guessedLetters):
        guessedLetters = word_updater(guessedLetters, word)
        word_printer(word, guessCount, guessedLetters)
    print("\n")
    if win_checker(word, guessedLetters):
        print("You won!")
        guessReset = guessReset - 1 # Decreases guessCount on a win to increase difficulty
    else:
        print(f"You lost! The word was {word}")
        guessReset += 1 # Increases guessCount on a loss to decrease difficulty
    if input("Would you like to play again? (y/n)").lower() == "n":
        break
    print("\n")

While I am pretty happy with it, in an attempt to avoid using globals (which I failed), I run a lot of variables through functions that don't use them other than to call other functions, and I was wondering if there was a better way to do this? Also is there any other layout or readability improvements I can make?

Link to code and included files.

\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Just a few possible improvements.


Variable Naming

According to PEP 8, variable names, including parameters, should be in snake_case, not camelCase. This will apply to all the improvements made below.

Type Hints

You can use type hints to display what types of parameters are accepted, if any, and what types, if any, are returned by the function. Observe:

def my_function(string: str, length: int) -> bool:
    return len(string) == length

This is a very short example, but displays both points in action. You can clearly see that string should be of type str, and length should be of type int. You can also see that the function returns a boolean (bool) value.

word_loader

This function can be improved to the following:

def word_loader() -> str:
    """Loads a random word from the word list."""
    with open("words.txt") as file:
        lines = file.readlines()
    return random.choice(lines).strip()

Instead of generating a random number within the length of the file, you can instead use random.choice to select a random value within the list. Then, you can use .strip() to remove the newline from the end of the word.

word_printer

This function can be improved to the following:

def word_printer(word: str, guess_count: int, guessed_letters: list) -> None:
    """Prints out underscores for unguessed letters and the letter for correctly guessed letters."""
    word_data = ["_" for _ in enumerate(word)]
    word_data = info_adder(word_data, guessed_letters)
    print(" ".join(word_data)) # Prints the list with each character separated by a space
    print(f"You have {guess_count} wrong guesses left")

You can use list comprehension to create the list word_data in one line. Note that None is being used as a type hint. None, returned from a function, can indicate a Null Object, or it may signal that the function doesn't really return anything.

win_checker

This function can be improved to the following:

def win_checker(word: str, guessed_letters: list) -> bool:
    """Checks if game has been won."""
    correct_letters = sum(1 if letter[1] in guessed_letters else 0 for letter in enumerate(word))
    return correct_letters == len(word)

Two main improvements have been made in this function.

One, the use of generators. This is paired with the use of sum(). What the first line does is sets 1 if the boolean condition (if letter[1] in guessed_letters), and if it isn't, 0. This goes through the entire generator (for letter in enumerate(word)). sum() then returns an integer value, representing the sum of the generator expression.

Two, returning boolean expressions. Instead of if x > y return True else return False, you can simply return x > y. Since x > y results in a boolean value, returning the expression results in retuning the value returned by the expression. I may have worded that weirdly, so here are three identical statements:

if x > y:
    return True
return False
if x > y:
    return True
else:
    return False
return x > y

These do the exact same thing. The last one is nicer and a lot shorter.

\$\endgroup\$
2
  • \$\begingroup\$ All good points; I'd go a step further in win_checker and just return sum(...) == len(word) (but I'd break up that generator expression with some linebreaks and comments). \$\endgroup\$
    – Samwise
    Jan 17, 2020 at 17:34
  • \$\begingroup\$ @SamStafford I think for readability's sake, knowing what that generator represents could be helpful. \$\endgroup\$
    – Linny
    Jan 17, 2020 at 20:01
1
\$\begingroup\$

General comments

  • The parameters you're passing in to almost every function collectively represent the current game state, which is perfectly fine for this style of programming which favors using functions over classes. Some might prefer modeling Hangman as a class with the game state, i.e. word and guessed_letters, as instance variables, but both styles are fine and work well for a simple game like this.
  • Some of the inline comments such as # Checks if a letter has been guessed before or # Initializes the variables are not helpful and actually clutter the code instead of making it easier to read. Ideally your code should be self-documenting if you have descriptive function/variable names and straightforward logic. Of course, this is sometimes easier said than done. For complex logic or lengthy/dense areas of code, you might need to explain what your code is doing. And if you think the design/implementation decisions you made might surprise your reader, including a comment explaining why you chose to write it that way could be helpful.
  • I'd recommend moving your game loop code (which is currently at the same level as your function definitions) under a if __name__ == "__main__": guard to prevent it from running the game if your file is imported as a module.
  • At the end is the final refactored version, but in the interest of time/brevity not all of the refactored parts are mentioned below.

word_loader

  • I would rename this to words_loader and change it to take a file path as a parameter and return a list of words.
  • Having the file path as a parameter gives you the flexibility to select from a different word list file than words.txt if you later decide you want to accept an arbitrary word list file as a command-line argument.
  • Returning a list of words from this function means that you only need to call this once per program invocation, i.e. you only need to read from the file once. Then in your top-level game loop pick from the list at random.
  • Example of how it might look after the above changes:
    def words_loader(filepath: str) -> List[str]:
        with open(filepath) as f:
            return f.read().strip().lower().split()
    

word_updater

  • A small bug: userData.lower() returns a copy of userData with all the cased characters converted to lowercase; it does not mutate userData in place. So when you have it on a line by itself, you are creating a copy of the string with all of its cased characters converted to lowercase, then immediately discarding that copy. To fix this, call lower() on the result of input() and store the result in user_data:
    user_data = input("What letter would you like to guess? ").lower()
    
  • guessCount does not need to be global if you make it a function parameter like you're doing with word and guessedLetters. But in this case I would recommend not tracking guessCount like a counter that you need to decrement manually. More on this below.
  • To simplify naming and make things less ambiguous, I propose renaming what you currently call guessReset as starting_lives to represent the number of starting lives the player has. The idea here is that for each incorrect guess, the player loses a life. And when lives_used == starting_lives, i.e. when the player has used up or consumed all of their lives, they lose the game.
  • We can calculate lives_used from word and guessed_letters:
    lives_used = len(set(guessed_letters) - set(word))
    
    set(guessed_letters) is the set of letters guessed so far, and set(word) is the set of letters that make up the target word. set(guessed_letters) - set(word) is the set difference between the two which gives us the set of incorrectly-guessed letters, i.e. the number of lives used.

info_adder and word_printer

  • Since word_printer is the only one using info_adder, I would just consolidate these into one function.
  • Instead of a List[str], it makes more sense to maintain guessed_letters as a Set[str] given that we don't want duplicate letters in this collection.
  • word_printer after the above changes:
    def word_printer(word: str, guessed_letters: Set[str],
                     starting_lives: int) -> None:
        lives_used = len(guessed_letters - set(word))
        print(" ".join(c if c in guessed_letters else "_" for c in word))
        print(f"You have {starting_lives - lives_used} wrong guesses left")
    

win_checker

  • The idea is to check if the game has been won, but we can't actually determine that without also passing in starting_lives. This function should ideally be able to tell us whether the player won, lost, or is still playing the game.
  • I think it would be useful to introduce a Result enumeration like the following:

    from enum import Enum, auto
    
    class Result(Enum):
        WIN = auto()
        LOSS = auto()
    

    Then win_checker's return type could be Optional[Result], where Result.WIN corresponds to a win, Result.LOSS corresponds to a loss, and None corresponds to the game still being in progress.

  • An example implementation:

    def win_checker(word: str, guessed_letters: Set[str],
                    starting_lives: int) -> Optional[Result]:
        word_letters = set(word)
        lives_used = len(guessed_letters - word_letters)
    
        if lives_used >= starting_lives:
            return Result.LOSS
        if word_letters.issubset(guessed_letters):
            return Result.WIN
        return None
    

Final refactored version:

#!/usr/bin/env python3

import random
from enum import Enum, auto
from typing import List, Optional, Set


class Result(Enum):
    WIN = auto()
    LOSS = auto()


def words_loader(filepath: str) -> List[str]:
    with open(filepath) as f:
        return f.read().strip().lower().split()


def word_updater(word: str, guessed_letters: Set[str]) -> Set[str]:
    while True:
        user_data = input("What letter would you like "
                          "to guess? ").strip().lower()
        if user_data in guessed_letters:
            print("You have guessed that letter before, "
                  "please use another letter.")
        elif len(user_data) != 1:
            print("Please enter only one letter.")
        elif not user_data.isalpha():
            print("Please only enter letters.")
        else:
            guessed_letters.add(user_data)
            return guessed_letters


def word_printer(word: str, guessed_letters: Set[str],
                 starting_lives: int) -> None:
    lives_used = len(guessed_letters - set(word))
    print(" ".join(c if c in guessed_letters else "_" for c in word))
    print(f"You have {starting_lives - lives_used} wrong guesses left")


def win_checker(word: str, guessed_letters: Set[str],
                starting_lives: int) -> Optional[Result]:
    word_letters = set(word)
    lives_used = len(guessed_letters - word_letters)

    if lives_used >= starting_lives:
        return Result.LOSS
    if word_letters.issubset(guessed_letters):
        return Result.WIN
    return None


def play(word: str, starting_lives: int) -> Result:
    guessed_letters: Set[str] = set()

    word_printer(word, guessed_letters, starting_lives)
    while True:
        guessed_letters = word_updater(word, guessed_letters)
        word_printer(word, guessed_letters, starting_lives)
        final_result = win_checker(word, guessed_letters, starting_lives)
        if final_result:
            return final_result


def wants_to_play_again() -> bool:
    while True:
        choice = input("Would you like to play again? (y/n) ").strip().lower()
        if choice in ("y", "n"):
            return choice == "y"


if __name__ == "__main__":
    starting_lives = 8
    words = words_loader("words.txt")
    while True:
        word = random.choice(words)
        final_result = play(word, starting_lives)

        print("\n")
        if final_result is Result.WIN:
            print("You won!")
            starting_lives -= 1
        else:
            print(f"You lost! The word was {word}")
            starting_lives += 1
        print("\n")

        if not wants_to_play_again():
            break
\$\endgroup\$
2
  • \$\begingroup\$ I did not put a file location because I don't always run it from the same location, and my teacher tests it on an online python interpreter so its easier without the file path. \$\endgroup\$
    – user211881
    Jan 22, 2020 at 20:19
  • 1
    \$\begingroup\$ @K00lman Makes sense, given that it's an assignment where you can make assumptions about input file names and locations. But from a design perspective, I still think having words_loader take in a file path as the parameter is strictly better than leaving words.txt hard-coded in the function body. If you leave it there hard-coded, the function will only ever work for that very specific filename in the current working directory. \$\endgroup\$
    – Setris
    Jan 22, 2020 at 22:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.