Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/

edited May 23, 2017 at 12:40

Once you have some functions in the file, you should use the if __name__ == '__main__' construction if __name__ == '__main__' construction, which allows the same file to be used both as a module and a script. Like so:

Once you have some functions in the file, you should use the if __name__ == '__main__' construction, which allows the same file to be used both as a module and a script. Like so:

added 1208 characters in body

Source Link

edited Sep 23, 2015 at 20:29

alexwlchan

edited Sep 23, 2015 at 20:29

alexwlchan

8.7k
1
23
55

tl;dr: This is pretty good for a beginner. It’s robust to bad input and plays smoothly. To improve it, you could use functions to make it more reusable, and I’ve suggested a bunch of small ways to tidy up your Python. But a high level it works well.

I’m just going to dive in, run the script, and see what happens. You didn’t provide a words.txt file, so I just used this small sample:

Overall, it seems pretty robust. I was unable to cause a traceback or exception by passing in dodgy input – kudos! 👍

(And yes, I passed in an emoji thumbs-up, and it handled it fine.)
At the risk of getting bludgeoned to death by Jeff Atwood, I found a pluralisation bug:
```
 Try again, you have only 1 lives left!
 So far you have used:
 ['b', 'c', 'd', 'e', 'f', 'i']
```
Should be "lives".
The program treats uppercase and lowercase letters as distinct. That’s not what I expect when I play Hangman – at least whenever I’m played it, I’m just guessing letters, not their associated case as well.
There are a few cases where the program just asks for the next letter, and doesn’t give a helpful message. When I guess a letter that I’ve already guessed (correct or not), it goes straight to prompting me for a new letter without explaining why.

That’s more adding polish than fixing a bug – but it would be nice.
I think a newline between guesses would make the output much easier to read. That’s a personal preference though, not a bug.

At a high-level, the problem with this code is that it isn’t very reusable. It’s good to break your code down into functions – that allows it to be reused within the same file, or across other files.

For example, this script only allows me to play a single game of Hangman. Having a play_hangman() function would allow you to keep playing Hangman until the player got bored. Here are some suggestions to get you started:

play_hangman(word, number_of_lives)
get_random_word_from_file(file_path) – letting me specify the file path means I could drop in another word list easily

I think the while loop is fine for this purpose.

Once you have some functions in the file, you should use the if __name__ == '__main__' construction , which allows the same file to be used both as a module and a script. Like so:

if __name__ == '__main__':
 play_hangman('pomegranate', number_of_lives=8)

The code inside that if block only runs if the script is run directly; if you access part of the file via an import statement, it doesn’t run. That means you can import from the file without having to play a game of Hangman first.

I’m just going to dive in, run the script, and see what happens. You didn’t provide a words.txt file, so I just used this small sample:

Overall, it seems pretty robust. I was unable to cause a traceback or exception by passing in dodgy input – kudos! 👍

(And yes, I passed in an emoji thumbs-up, and it handled it fine.)
At the risk of getting bludgeoned to death by Jeff Atwood, I found a pluralisation bug:
```
 Try again, you have only 1 lives left!
 So far you have used:
 ['b', 'c', 'd', 'e', 'f', 'i']
```
Should be "lives".
The program treats uppercase and lowercase letters as distinct. That’s not what I expect when I play Hangman – at least whenever I’m played it, I’m just guessing letters, not their associated case as well.
There are a few cases where the program just asks for the next letter, and doesn’t give a helpful message. When I guess a letter that I’ve already guessed (correct or not), it goes straight to prompting me for a new letter without explaining why.

That’s more adding polish than fixing a bug – but it would be nice.
I think a newline between guesses would make the output much easier to read. That’s a personal preference though, not a bug.

I’m just going to dive in, run the script, and see what happens. You didn’t provide a words.txt file, so I just used this small sample:

Overall, it seems pretty robust. I was unable to cause a traceback or exception by passing in dodgy input – kudos! 👍

(And yes, I passed in an emoji thumbs-up, and it handled it fine.)
At the risk of getting bludgeoned to death by Jeff Atwood, I found a pluralisation bug:
```
 Try again, you have only 1 lives left!
 So far you have used:
 ['b', 'c', 'd', 'e', 'f', 'i']
```
Should be "lives".
The program treats uppercase and lowercase letters as distinct. That’s not what I expect when I play Hangman – at least whenever I’m played it, I’m just guessing letters, not their associated case as well.
There are a few cases where the program just asks for the next letter, and doesn’t give a helpful message. When I guess a letter that I’ve already guessed (correct or not), it goes straight to prompting me for a new letter without explaining why.

That’s more adding polish than fixing a bug – but it would be nice.
I think a newline between guesses would make the output much easier to read. That’s a personal preference though, not a bug.

play_hangman(word, number_of_lives)
get_random_word_from_file(file_path) – letting me specify the file path means I could drop in another word list easily

I think the while loop is fine for this purpose.

Once you have some functions in the file, you should use the if __name__ == '__main__' construction , which allows the same file to be used both as a module and a script. Like so:

if __name__ == '__main__':
 play_hangman('pomegranate', number_of_lives=8)

Source Link

answered Sep 23, 2015 at 20:24

alexwlchan

answered Sep 23, 2015 at 20:24

alexwlchan

8.7k
1
23
55

I’m just going to dive in, run the script, and see what happens. You didn’t provide a words.txt file, so I just used this small sample:

american
captain
dictionary
fish
hangman
letter
list

Comments:

Overall, it seems pretty robust. I was unable to cause a traceback or exception by passing in dodgy input – kudos! 👍

(And yes, I passed in an emoji thumbs-up, and it handled it fine.)
At the risk of getting bludgeoned to death by Jeff Atwood, I found a pluralisation bug:
```
 Try again, you have only 1 lives left!
 So far you have used:
 ['b', 'c', 'd', 'e', 'f', 'i']
```
Should be "lives".
The program treats uppercase and lowercase letters as distinct. That’s not what I expect when I play Hangman – at least whenever I’m played it, I’m just guessing letters, not their associated case as well.
There are a few cases where the program just asks for the next letter, and doesn’t give a helpful message. When I guess a letter that I’ve already guessed (correct or not), it goes straight to prompting me for a new letter without explaining why.

That’s more adding polish than fixing a bug – but it would be nice.
I think a newline between guesses would make the output much easier to read. That’s a personal preference though, not a bug.

Finally, here are some minor line-by-line comments and/or nitpicks:

You’re pretty good for PEP 8 compliance. One line is a bit long, and you need a bit of extra spacing around your comments, but otherwise fine.
The wordlist variable is unused.
When you choose the word, you open the words.txt file, but don't close it again. You could add an explicit close, but a better approach is to use the with keyword when opening files, like so:
```
 with open("words.txt") as wordsfile:
 word = random.choice(wordsfile.readlines())
```
The length variable is only used twice (and I’m going to get rid of the second usage shortly), so I’d remove it and just use it directly.
The way you’ve constructed the mock_word list is a little strange. You could go straight to it in one line:
```
 mock_word = ['_'] * len(word)
```
Also note that the word "mock" has a special meaning in programming, so I’d suggest picking a different name for this variable – perhaps player_guess?
Don’t be stingy with variable names – characters are cheap. Names like used_lets and let can become used_letters and guessed_letter. It makes your program easier to read.

Now I’m diving into the heart of the program: the while loop that processes the player guess.

The first if branch is fine. However, you’re only checking that I don’t enter too many characters; you don’t check that I enter enough characters. This loop could be improved by checking for an incorrect number of characters, and printing a generic message:
```
 if len(letter) != 1:
 print("Please enter a single character.")
```
In the elif branch, a common way to decrement a variable is to write lives -= 1. That’s a bit more compact.

In the print statement telling me how many lives I have left, you’re using the %r token. That’s fairly generic – it just prints the string representation of whatever gets passed in. Since you know that the lives variable is an integer, I think it’s better to use the more explicit token %d. As I read the string, I know what sort of data will appear here.
In the else branch, a more Pythonic approach than iterating over the indexes is to use enumerate(), which iterates over the index and the array element together. Like so:
```
 for i, letter in enumerate(word):
 if guessed_letter == letter:
 player_guess[i] = letter
```

lang-py