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
1 Answer 1
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 overmain_word
andcovered_word
, simultaneously, taking one character from eachfor m, c in ...
takes the characters from each string, and calls themm
(for main) andc
(for covered).m if m == user_guess else c
evaluates to either them
(the newly guessed character ifm == user_guess
is true) orc
(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.