3
\$\begingroup\$

I am fairly new to coding and decided to test my skills and make a program from scratch on my own. I chose to make a Hangman game, it's pretty straight forward. The code reads a file consisting of ~40,000+ words to choose from when generating a word for the user. The user inputs the number of attempts and the minimum length of a word they would prefer. As the player guesses, the word is displayed with only the correct guesses displayed to the user. The incorrect guesses are shown to the user as they are entered. Fairly simple, yet I feel like my code is confusing so I was just curious if anyone had any tips on what I could fix/improve to make my code more readable and run more efficiently. Any feedback is appreciated.

import random

word_file = 'popular.txt'

word_list = open(word_file).read().splitlines()

def get_attempts():
    """
    returns # attempts user wants
    """
    while True:
        try:
            attempts = int(input("How many incorrect attempts do you want? [1-25] "))
        except:
            print('There was a error. Enter an interger between 1 and 25!\n')
            continue
        else:
            return attempts

def get_min_len():
    """
    returns minimum word length
    """
    while True:
        try:
            min_len = int(input("What minimum word length do you want[4-16] "))
        except:
            print('There was a error. Enter an integer between 4 and 16!\n')
            continue
        else:
            if min_len < 4 or min_len > 16:
                print("Enter a integer between 4 and 16!\n")
                continue
            else:
                return min_len

def pick_word(minlen):
    # pick random word from word list

    random_word = random.choice(word_list)

    while len(random_word) < minlen:
        random_word = random.choice(word_list)

    return random_word

def fill_word(main_word, covered_word, user_guess):
    """
    fills in hidden word as user guesses correctly
    """
    covered_word_list = list(covered_word)

    for idx in range(0, len(main_word)):

        if main_word[idx] == user_guess:
            covered_word_list[idx] = user_guess

    covered_word = ''.join(covered_word_list)
    return covered_word

def display_info(covered_word, atmps, incorrect):

    print("Word: {}".format(covered_word))
    print("Attempts Remaining: {}".format(atmps))
    print("Previous Guesses: {}".format(incorrect))

def get_guess():
    """
    error/exception handling for user input
    """

    while True:
        try:
            user_guess = str(input('Choose a letter: '))
        except:
            print("There was a Error. Enter a letter!\n")
            continue
        else:
            if not user_guess.isalpha():
                print("Guess MUST BE a STRING!\n")
                continue
            elif len(user_guess) != 1:
                print("Enter ONLY ONE letter!\n")
                continue
            else:
                return user_guess

def check_guess(main_word, user_guess):

    if guess in main_word:
        print("{} is in the word!\n".format(user_guess))
    else:
        print("{} is not in the word!\n".format(user_guess))

def start_game():
    while True:
        try:
            start = input("Are you ready to play? Enter 'Yes' or 'No': ")

        except:
            print("There was an error. Try again.\n")
            continue
        else:
            if start.isspace():
                print("Enter either 'Yes' or 'No'!")
                continue
            if start[0].lower() == 'y':
                play = True
                return play
            elif start[0].lower() == 'n':
                play = False
                return play
            else:
                print("There was an error, try again.\n")
                continue

def replay():
    while True:
        try:
            replay = input("Do you want to play again. Enter 'Yes' or 'No': ")

        except:
            print("There was an error. Try again.\n")
            continue
        else:
            if replay[0].lower() == 'y':
                play = True
                return play
            elif replay[0].lower() == 'n':
                play = False
                return play
            else:
                print("There was an error, try again.\n")
                continue

GAMEPLAY

play = start_game()  # ask user if they are ready return bool

while play:

    attempts = get_attempts()       # get # of attempts user wants
    min_len = get_min_len()         # get min len of word

    print("Selecting a word...\n")
    word = pick_word(min_len)       # choose word <= min_len
    final_word = word               # store word has another variable
    hidden_word = '*' * len(word)   # create str of '*' == len(word)

    guess_list_w = []       # list of incorrect guesses
    guess_list_c = []       # list of correct guesses

    while word != hidden_word:

        display_info(hidden_word, attempts, guess_list_w)     # display information for user

        guess = get_guess()           # get a guess from user

        if guess in word:
            if guess not in guess_list_w and guess not in guess_list_c:
                print("{} is in the word!\n".format(guess))      # if correct guess
                hidden_word = fill_word(final_word, hidden_word, guess)
                guess_list_c.append(guess)
                continue
            elif guess in guess_list_c:       # if guess is in word but already guessed try again
                print("{} has been guessed already! Try Again!\n".format(guess))
                continue
        elif guess in guess_list_w:             # if guess is not in word but already guessed
            print("{} has been guessed already! Try Again!\n".format(guess))
            continue
        else:                                   # if guess hasnt been guessed and is not in the word
            print("{} is not in the word!\n".format(guess))  # if incorrect guess
            guess_list_w.append(guess)
            if attempts == 1:   # check num of attempts left; if 0 break
                break
            else:
                attempts -= 1

    # Winning Scenario
    if hidden_word == word:
        print("You Win! You guesses the correct letters!")
        print("The word was {}\n\n".format(final_word))
    else:
        # Losing Scenario
        print("You Lose! You're out of attempts!")
        print("The word was {}\n\n".format(final_word))

    play = replay()     # ask player to play again
\$\endgroup\$
2
\$\begingroup\$

Getting Input

def get_attempts():
    """
    returns # attempts user wants
    """
    while True:
        try:
            attempts = int(input("How many incorrect attempts do you want? [1-25] "))
        except:
            print('There was a error. Enter an interger between 1 and 25!\n')
            continue
        else:
            return attempts

You've got some unnecessary statements here. If you did not execute continue in the except clause, what would happen? The control wouldn't go into the else: clause, so it would reach the end of the while True: body, and loop around to the top ... just like executing in the case of executing the continue. So you can remove that statement.

Why the else: clause? It is executed when execution of the try: block doesn't go into the except: clause. So, if the statements from the else: clause are moved into the try block, the execution would be similar. The only difference is that an exception raised during the else: clause won't go into the except:, but what exception is likely to be raised by return attempts? None. So why complicate the try: ... except: with an else:? Move return attempts into the body of the try:.

What exceptions are likely to be raised in the try: block, anyway? A ValueError, when the input can't be parsed as a integer. What else? Maybe EOFError if the user tries to run the program with a test case redirected from standard input? You'd catch the EOFError, print a message, loop back and try for more input, and get another EOFError. Endless loop. Yuck. But it gets worse. The user, upon seeing thousands of repeated lines will press Ctrl-C to stop the program, which raises (wait for it) a KeyboardInterrupt exception, which will get caught, a message will be printed, and then execution will loop around and ask for more input. You can't break out of the program. Don't catch all exceptions; catch only the ones you actually expect.

If the user types in 26, which is not in the range of 1-25, the program ... happily accepts the input. You aren't testing the value.

Reworked code (version 1):

def get_attempts():
    """
    return # of attempts user wants
    """
    while True:
        try:
            attempts = int(input("How many incorrect attempts do you want? [1-25] "))
            if 1 <= attempts <= 25:
                return attempts
        except ValueError:
            print('There was an error.  ', end='')

        print('Enter an integer between 1 and 25!\n')

The next function get_min_len() looks eerily familiar. You could make exactly the same changes described above. Which sounds like WET code, where WET stands for Write Everything Twice. The opposite of WET is DRY: Don't Repeat Yourself. So let's dry up this pair of functions.

def input_int(prompt, minimum, maximum):
    """Ask user for an integer within a restricted range"""

    prompt += f" [{minimum}-{maximum}] "

    while True:
        try:
            value = int(input(prompt))
            if minimum <= value <= maximum:
                return value
        except ValueError:
            print('There was an error.  ', end='')

        print(f'The value must be an integer between {minimum} and {maximum}\n')

def get_attempts():
    return input_int("How many incorrect attempts do you want?", 1, 25)

def get_min_len():
    return input_int("What minimum word length do you want?", 4, 16)

Random Word

Continuing the WET -vs- DRY theme:

def pick_word(minlen):
    # pick random word from word list

    random_word = random.choice(word_list)

    while len(random_word) < minlen:
        random_word = random.choice(word_list)

    return random_word

You have two identical statements: random_word = random.choice(word_list). You need the first, because you want to test len(random_word) < minlen in the while loop condition, so you need to have defined random_word. But do you need to actually select a random word to do it? Or could any sufficiently short value cause the loop to be entered and executed?

    random_word = ""

    while len(random_word) < minlen:
        random_word = random.choice(word_list)

How many times do you expect to loop until you choose a word of sufficient length? 2, maybe 3 times if you are unlucky? What if the user has requested long words, where minimum length is 16? There might only be a fraction of a percent of words that long in the list of 40,000 words. It might loop many, many times before it stumbles upon "antidisestablishmentarianism".

    long_words = [words for word in word_list if len(word) >= minlen]

    if len(long_words) == 0:
        raise RuntimeError("No words long enough in word list!")

    random_word = random.choice(long_words)

Now we've guaranteed we can pick a word in one try, so the while ...: loop is unnecessary.

Iterating over two lists at once

The fill_word() method iterates over both covered_word and main_word, searching for letters which match user_guess in main_word and filling them into the covered_word.

It does so with covered_word_list and idx as temporaries.

Python allows iterating over 2 (or more!) iterable objects at once, using the zip() function. This makes implementing the function almost trivial.

def fill_word(main_word, covered_word, user_guess):
    """
    fills in hidden word as user guesses correctly
    """

    return ''.join(m if m == user_guess else c for m, c in zip(main_word, covered_word))

That's a pretty dense return statement. Let's break it down.

  • String are iterable objects. When you iterate over a string, you get the characters one at a time.
  • zip(main_word, covered_word) iterates over main_word and covered_word, simultaneously, taking one character from each
  • for m, c in ... takes the characters from each string, and calls them m (for main) and c (for covered).
  • m if m == user_guess else c evaluates to either the m (the newly guessed character if m == user_guess is true) or c (the original character from the covered string).
  • ''.join(...) takes each character as they are generated above, from the corresponding pairs of characters, and joins them back into one long string.

Variable names and f-strings

def display_info(covered_word, atmps, incorrect):

    print("Word: {}".format(covered_word))
    print("Attempts Remaining: {}".format(atmps))
    print("Previous Guesses: {}".format(incorrect))

covered_word is fine, incorrect barely clears the bar, but atmps? What is that? Use clearer variable names. Please.

I've used them above, but I'll call out attention to them here. Formatted strings, or f-strings, are a new feature in Python, which allows you to embed variable names right into strings. Instead of having many characters separating a {} code in a format string, and the variables being passed to the .format(...) call, you can directly tell what variable is being substituted where into a string. You just need to prefix the string with an f.

def display_info(covered_word, attempts_remaining, incorrect_guesses):

    print(f"Word: {covered_word}")
    print(f"Attempts Remaining: {attempts_remaining}")
    print(f"Previous Guesses: {incorrect_guesses}")

String input

input() returns a string. There is no need to wrap str(...) around an input() call. Since no conversion is being done, there won't be any exception that may arise. (EOFError and KeyboardInterrupt not withstanding, but you don't want to catch either of those and loop to try to get input again!)

So, get_guess() can be simplified. Remove the try: ... except: entirely. Remove the unnecessary continue statements. My review is bordering on WET. I need to stop repeating myself. These points apply to get_guess(), start_game(), and replay(). You are asking "Yes/No" in two locations? Like the input_int() above, perhaps you want a ask_yes_no(prompt) function.

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