5
\$\begingroup\$

To put it simply, I'm making a hangman game and it works as it is, it correctly outputs what I want it to, and I have no problem with submitting it now. I only wish to make it more efficient, and I doubt it's at max efficiency as it is. So here it is.

"""
This code runs a hangman game for the user.
"""
# This function gets the user's guess for the letter and determines if its correct
def get_guess():
    global user_guess
    while True:
        user_guess = input("Guess a letter: ")
        if user_guess.isnumeric():
            print("That is not a letter!")
        elif len(user_guess) > 1:
            print("You can only enter one letter!")
            continue
        elif user_guess.islower() == False:
            print("Remember to enter a lowercase letter, but I'll swap it for you this time :)")
            return user_guess.lower()
        else:
            return user_guess

# This function updates the dashes that the code outputs, this function displays the progress
# of the user's guess for the secret word
def update_dashes(secret_word, dashes, user_guess):
    for i in range(len(secret_word)):
        if secret_word[i] == user_guess:
            dashes  = dashes[:i] + user_guess + dashes[i + 1:]
            return dashes

# This imports the random function
import random
# This list defines what words the random.choice method can select from for hangman 
words = ["jazz", "hello", "bird", "birds", "peanut", "butter", "cake", "hallway"]
# This secret_word defines a random word from the list for the user to guess
secret_word = random.choice(words)
# This variable defines the progress meter of dashes based on the length of the random word
dashes = "-" * len(secret_word)
# This variable defines the number of tries the user starts with and has left
tries = 10
# This while statement updates the user's progress for guessing the word
while tries > 0 and dashes != secret_word:
    print(dashes)
    guess = get_guess()
    dashes = update_dashes(secret_word, dashes, user_guess)
    if guess in secret_word:
        print("That letter is in the secret word!")
    else:
        print("That letter is not in the secret word.")
        tries = tries - 1
         print("You have " + str(tries) + " tries left.")
    
# This if statement determines if the user has won or lost based on their tries or if the 
# progress meter is full
if tries == 0:
    print("You ran out of guesses, the word was " + secret_word + ".")
    print("--------------------------YOU LOSE----------------------------")
elif dashes == secret_word:
    print(dashes)
    print("You guessed correctly, the word was " + secret_word + "!")
    print("--------------------------YOU WIN!----------------------------")
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Welcome to CodeReview! Your general structure is already pretty good and the comments made the code easy to read. Here are some key suggestions and explanations. The full code can be found at the bottom.


Comments

Comments should generally not (have to) explain every single line, especially really simple lines like an import:

# This imports the random function
import random

Any Python developer that reads this will know what the line does. Adding a comment here significantly increases the time needed to read and understand the code, so I'd advise against simple comments like this.


f-Strings

f-Strings offer a convenient and readable way to print text that includes variables:

print("You have " + tries + " tries left.")

becomes

print(f"You have {tries} tries left.")

dashes and update_dashes

Since you need to modify dashes multiple times I would suggest a mutable data type instead of a str, e.g. a list.

dashes = ["-"] * len(secret_word)

Since it's now a list, we would need to print it by calling "".join(dashes). We'll put that in its own seperate __str__ function in the next point.

This allows for a more concise definition of update_dashes(...):

def update_dashes(secret_word, dashes, user_guess):
    for index, letter in enumerate(secret_word):
        if letter == user_guess:
            dashes[index] = user_guess

Multiple things to notice here:

  1. dashes is modified in-place, therefore we don't need to return it. Passing it to the function is enough.
  2. Using enumerate will give us index and letter for all letters in secret_word. This approach is more pythonic than iterating over a range and accessing the elements by their index.
  3. We do not immediately return once we find a matching letter. This is important because we can have multiple occurences of the same letter in secret_word.

class Dashes

I would say dashes combines enough unique logic, for it to make sense to make it a standalone-class. This comes down to some personal preference, but I find it improves the code structure here.

class Dashes:
    PLACEHOLDER = "-"

    def __init__(self, secret_word):
        self.secret_word = secret_word
        self.letters = [self.PLACEHOLDER] * len(secret_word)

    def update(self, guess):
        for index, letter in enumerate(self.secret_word):
            if letter == guess:
                self.letters[index] = guess

    def is_completed(self):
        return self.PLACEHOLDER not in self.letters

    def __str__(self):
        return "".join(self.letters)

As you can see, Dashes now handles a lot of useful logic for the rest of our program.


Complete code

Some other points worth mentioning:

  1. You should avoid using global variables as far as possible. Global variables aren't good practice, as they can lead to some unexpected behaviour / confusing bugs.
  2. Wrapping your program's main functionality into a main() function is generally good practice. Wrapping your call to main() into a if __name__ == '__main__': condition is also a good idea. You can read more about it here.
  3. For if-conditions that break or continue a loop or return from a function, it often improves readability to move them into seperate conditions instead of an if ... elif ... elif ... else construct.
  4. user_guess.islower() == False is equivalent to not user_guess.islower(), and even better user_guess.isupper().
  5. When checking if user input is not a letter, you should use not user_guess.isalpha() instead of only checking for numbers with user_guess.isnumeric().
  6. tries = tries - 1 is equivalent to tries -= 1.
  7. Imports should usually be found at the top of your code file.
  8. We only need to update dashes if the user_guess is in secret_word.
  9. The length of secret_word will probably never be performance-critical. In general, if we need to perform multiple (performance-critical) membership tests (e.g. letter in word), sets provide lookups in constant instead of linear time.

Here's the full code without comments:

from random import choice


class Dashes:
    PLACEHOLDER = "-"

    def __init__(self, secret_word):
        self.secret_word = secret_word
        self.letters = [self.PLACEHOLDER] * len(secret_word)

    def update(self, guess):
        for index, letter in enumerate(self.secret_word):
            if letter == guess:
                self.letters[index] = guess

    def is_completed(self):
        return self.PLACEHOLDER not in self.letters

    def __str__(self):
        return "".join(self.letters)


def get_guess():
    while True:
        user_guess = input("Guess a letter: ")

        if not user_guess.isalpha():
            print("That is not a letter!")
            continue

        if len(user_guess) != 1:
            print("You can only enter one letter!")
            continue

        if user_guess.isupper():
            print("Remember to enter a lowercase letter, but I'll swap it for you this time :)")
            return user_guess.lower()

        return user_guess


def main():
    words = ["jazz", "hello", "bird", "birds", "peanut", "butter", "cake", "hallway"]
    secret_word = choice(words)

    dashes = Dashes(secret_word=secret_word)

    tries = 10

    while tries > 0 and not dashes.is_completed():
        print(dashes)
        user_guess = get_guess()

        if user_guess in secret_word:
            print("That letter is in the secret word!")
            dashes.update(user_guess)
        else:
            print("That letter is not in the secret word.")
            tries -= 1
            print(f"You have {tries} tries left.")

    if dashes.is_completed():
        print(dashes)
        print(f"You guessed correctly, the word was {secret_word}!")
        print("--------------------------YOU WIN!----------------------------")
    else:
        print(f"You ran out of guesses, the word was {secret_word}.")
        print("--------------------------YOU LOSE----------------------------")


if __name__ == "__main__":
    main()
\$\endgroup\$

Your Answer

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

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