I've been trying to practice more Python recently so I decided to try and make different simple text-based game programs. I'd hope anybody could help me see how to make the code neater or more efficient in any aspect.
import random
words = []
guessed_letters = []
word_guess = []
joined_word = None
def fill_list():
f = open('words.txt', 'r')
for line in f:
if len(line.strip()) > 3:
words.append(line.strip())
f.close()
def reset(secret):
for letter in secret:
word_guess.append('_')
def random_word(word_list):
chosen = random.choice(word_list)
return chosen
def guess_letter():
while True:
letter = input('Guess a letter a-z: ').lower().strip()
if len(letter) != 1:
print('Enter a single character only')
elif not letter.isalpha():
print('Enter only alphabetical characters')
elif letter in guessed_letters:
print('Already guessed {}'.format(letter))
else:
return letter
def print_opening():
print('Welcome to Hangman!\n'
'Try to guess a random word one letter at a time\n'
'Good luck and have fun!\n')
def play_again():
again = input().lower()
if again.lower().startswith('y'):
return True
return False
def play():
fill_list()
tries = 5
while True:
print_opening()
secret_word = random_word(words)
reset(secret_word)
while tries != 0 and '_' in word_guess:
print('You have {} tries left'.format(tries))
joined_word = ''.join(word_guess)
print(joined_word)
player_guess = guess_letter()
for index, letter in enumerate(secret_word):
if player_guess == letter:
word_guess[index] = player_guess
guessed_letters.append(player_guess)
if player_guess not in secret_word:
print('Wrong!')
tries -= 1
if '_' not in word_guess:
print('You win!\n{} was the word!\n'.format(secret_word))
else:
print('You lose!\nThe word was {}'.format(secret_word))
print('Would you like to play another game? y/n')
if play_again():
word_guess.clear()
tries = 5
else:
print('Thanks for playing!')
break
if __name__ == '__main__':
play()
2 Answers 2
Try to avoid global variables. For example words
doesn't need to be a global. Instead, return a list of words from fill_list()
, directly in the play()
method:
def fill_list():
words = []
f = open('words.txt', 'r')
for line in f:
if len(line.strip()) > 3:
words.append(line.strip())
f.close()
return words
...
def play():
words = fill_list()
...
Use a resource manager (aka a with
statement) for opening closable resources. That takes the burden of calling .close()
off of you, and ensures the resource is properly closed even when exceptions occur:
def fill_list():
words = []
with open('words.txt', 'r') as f:
for line in f:
if len(line.strip()) > 3:
words.append(line.strip())
return words
Avoid repeating the same calculations. You strip
the line
to check its length, then you strip
the line
again to append
it to words
. Save the stripped line in a variable.
def fill_list():
words = []
with open('words.txt', 'r') as f:
for line in f:
stripped = line.strip()
if len(stripped) > 3:
words.append(stripped)
return words
Use list comprehension.
def reset(secret):
word_guess = ['_' for letter in secret]
Or without the global variable:
def reset(secret):
return ['_' for letter in secret]
...
def play():
...
word_guess = reset(secret_word)
...
f-strings (f''
) are a new feature as of Python 3.6. You can use them to avoid .format()
statements by embedding variables directly in the strings. Eg)
print('Already guessed {}'.format(letter))
becomes
print(f'Already guessed {letter}')
Don't repeat yourself. You have tries = 5
at the top of play()
and the bottom if the user elects to play again. If you moved tries = 5
inside the while True:
loop, at the top of the loop, you'd just need it once.
Similarly for word_guess.clear()
, you should move it to the beginning of the loop ... except it can be removed entirely because of the word_guess = reset(secret_word)
change above.
Bugs
If words.txt
contains uppercase characters, hyphens, apostrophe's, etc., the user will not be able to guess the word. You should:
- normalize the
secret_word
by calling.lower()
on it. replace only letters with the underscore, leave hyphens, apostrophes, spaces alone.
def reset(secret): return ['_' if letter.isalpha() else letter for letter in secret]
One thing I would do is wrap this up in a hangman class. That way you could just toss away the hangman instance for a new one if the user ever decided to play again rather than resting all global variables. It would also decouple the handling of user input from the actual game. If the game size would grow this would make catching bugs a lot easier.
But lets focus on some smaller things that are easier to change
chosen = random.choice(word_list)
return chosen
Here your variable has no purpose, I would do
return random.choice(word_list)
Similar kind of story here
if again.lower().startswith('y'):
return True
return False
to
return again.lower().startswith('y')
Next is avoid magic constants at all costs, for example
if len(line.strip()) > 3:
What is 3? Why is 3 important here? You should replace this with a global ALL_CAPS variable to represent what 3 is.