Python Hangman feedback

Question

Just looking for some feedback on a hangman script. It works perfectly fine; I'm just trying to master the Python language, and the best place way to get better is to ask the true masters!

import random
UNKNOWN_CHARACTER = "*"
GUESS_LIMIT = 7
words = []
def load_words():
 global words
 with open("dictionary.txt") as file:
 words = file.readlines()
 stripped = []
 for word in words:
 stripped.append(word.strip())
 words = stripped
def play():
 word = random.choice(words)
 solved = False
 constructed = ""
 guessed = []
 guess = ""
 for i in range(0, len(word)):
 constructed += UNKNOWN_CHARACTER
 while not solved and len(guessed) < GUESS_LIMIT:
 print("\n" + str(GUESS_LIMIT - len(guessed)) + " errors left...")
 print(constructed)
 valid_guess = False
 while not valid_guess:
 guess = input("Guess a letter: ").lower()
 if len(guess) == 1:
 if guess not in guessed and guess not in constructed:
 valid_guess = True
 else:
 print("You've already guessed that letter!")
 else:
 print("Please guess a single letter.")
 if guess in word:
 new_constructed = ""
 for i in range(0, len(word)):
 if word[i] == guess:
 new_constructed += guess
 else:
 new_constructed += constructed[i]
 constructed = new_constructed
 else:
 guessed.append(guess)
 solved = constructed == word
 print("\n" + word)
def main():
 load_words()
 keep_playing = True
 while keep_playing:
 play()
 keep_going = input("Continue playing? (y/n): ").lower()
 if keep_going not in ["yes", "y"]:
 keep_playing = False
if __name__ == "__main__":
 main()

Excerpt of dictionary.txt:

logorrheic
logos
logotype
logotypes
logotypies
logotypy
logroll
logrolled
logroller
logrollers
logrolling
logrollings
logrolls
logs
logway
logways
logwood
logwoods
logy
loin
loincloth
loincloths
loins
loiter
loitered
loiterer
loiterers
loitering
loiters
loll
lollapalooza
lollapaloozas
lolled
loller
lollers

• SylvainD: Can you provide an example of dictionnary.txt ?

Answer 1

List comprehension is a very neat way to construct lists in Python. For instance :

stripped = []
for word in words:
 stripped.append(word.strip())

becomes

stripped = [word.strip() for words in word]

(and then you can get rid of the stripped variable altogether in your code).

---

The pythonic way to loop over something is not to use range and len (except if you really have to). For instance, we have first :

for i in range(0, len(word)):
 constructed += UNKNOWN_CHARACTER

which can be written :

for i in word:
 constructed += UNKNOWN_CHARACTER

Then one might argue that you can use the * operator here to write :

constructed = UNKNOWN_CHARACTER * len(word)

---

Sometimes, you might think that you need to use range and len because you need the index corresponding to your iteration. This is what enumerate is for. For instance :

 new_constructed = ""
 for i in range(0, len(word)):
 if word[i] == guess:
 new_constructed += guess
 else:
 new_constructed += constructed[i]

becomes :

 new_constructed = ""
 for i,l in enumerate(word): # i in the index, l is the letter
 if l == guess: # note that we don't need to get the i-th element here
 new_constructed += l # I prefer l to guess here because it's shorter :-P
 else:
 new_constructed += constructed[i]

What would be really awesome here would to be to be able to iterate over 2 containers in the same time. zip allows you to do such a thing.

 new_constructed = ""
 for w,c in zip(word,constructed):
 if w == guess:
 new_constructed += w
 else:
 new_constructed += c

Then, it's pretty cool because we can make things slightly more concise :

 new_constructed = ""
 for w,c in zip(word,constructed):
 new_constructed += w if w == guess else c

If you want to play it cool and re-use list comprehension (or even generator expression (I use fancy words so that you can google then if required)) : you build some kind of list of characters/strings and join them together with join.

 new_constructed = ''.join((w if w == guess else c for w,c in zip(word,constructed)))

This being said, I probably wouldn't build that string this way but I just pointed to point this out so that you can discover new things.

---

Usually, global variables are frowned upon because they make things hard to track. In your case, it is not so much of an issue but let's try to split the logic in smaller parts. It makes things easier to understand and to test too.

Here, we just need to return the list of words from load_words.

---

It's interesting to store the letters that have been guessed. However, a list (build with [] and populated with append()) might not be the right container for this. What you really want at the end of the day is just to be able to tell quickly whether some letter has been guessed already. This is what sets are for.

---

You could store all guesses or just wrong guesses. You've decided to store only wrong guesses (as right guesses can be deduced from the constructed string).

Personnally, I'd rather save all guesses on one hand and the number of wrong guesses on the other hand at it might make things a bit easier later on. I am not saying that what you did was wrong at all, I just want to show a different way to do.

Also, I'm trying to split the logic used to

1. display what has been found away from the logic used to
2. know if a letter has been guessed
3. know if the character has been fully found.

At the moment, the variable constructed in used for these 3 things and can make things hard to understand.

At the end, here is what I came up with :

#!/usr/bin/python
import random
UNKNOWN_CHARACTER = "*"
GUESS_LIMIT = 7
def load_words():
 words = []
 with open("dictionary.txt") as file:
 words = file.readlines()
 return [word.strip() for word in words]
def play(word):
 nb_wrong = 0
 guesses = set()
 while nb_wrong < GUESS_LIMIT:
 print("\n" + str(GUESS_LIMIT - nb_wrong) + " errors left...")
 print(''.join(c if c in guesses else UNKNOWN_CHARACTER for c in word))
 while True:
 guess = input("Guess a letter: ").lower()
 if len(guess) == 1:
 if guess in guesses:
 print("You've already guessed that letter!")
 else:
 guesses.add(guess)
 break # stop when we have a valid guess 
 else:
 print("Please guess a single letter.")
 if all(c in guesses for c in word):
 break # stop when all characters are found
 print("\n" + word)
def main():
 words = load_words()
 while True:
 play(random.choice(words))
 if input("Continue playing? (y/n): ").lower() not in ["yes", "y"]:
 break
if __name__ == "__main__":
 main()

(It's highly not tested but the point was more to explain you what I did than to show you a working program)

• Chiggs: very minor tweak - I would put the single character test first: if len(guess) != 1: print("Please guess a single letter."); continue which would save an indentation level for the non-error case and potentially make play more readable.
• SylvainD: I guess it's a matter of personal preference. I prefer it the way it currently is but I understand your point.