3
\$\begingroup\$

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()
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jun 4, 2019 at 22:11
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

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]
    
answered Jun 5, 2019 at 2:05
\$\endgroup\$
2
\$\begingroup\$

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.

answered Jun 5, 2019 at 2:03
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.