HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

“Hangman” game in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamepythonhangman

Problem

I have been using Python 3 to learn programming, and this is a very basic task, but I want to gain good programming patterns/habits at the very beginning.

import sys
import random

words_list = ['russia', 'china', 'usa', 'canada', 'ghana'] #pre-made list of our words to guess
accepted_input = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
                    'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
word_guess = random.choice(words_list)
word_to_guess = []
for i in range(0, len(word_guess)):
    word_to_guess.append(word_guess[i])
currently = []
for i in word_to_guess:  # fill in our currently guessed list with "_"
    currently.append("_")
guess_letters = []
guesses = 0

#make sure the program will work if we run it using old versions of Python
if sys.version_info[0]  5: #game over you lost
            hangman_graphic(guesses)
            break
        elif "_" in currently: #keep playing
            hangman_graphic(guesses)
        else: #game over you WON
            hangman_graphic(-1)
            break

def hangman():
    hangman_graphic(guesses)
    guess_input()

if __name__ == "__main__":
    hangman()
    print("END!")

Solution

Defining the list of words directly in the program makes it very difficult to add more words later. A better way would be to have a file holding them, and an even better way would be to grab the words off the internet. The best way would be to have a file with the words, and try to update the file from the internet. Of course, no internet would mean no extra words, but that's why you have the file as backup.

It's great that you have an accepted_input, but that isn't the best way to define it. A better way would be

from string import ascii_letters # as accepted_input


Note that Python 2 uses letters, not ascii_letters.

Speaking of Python 2, you have a slight problem with your backwards-compatability. Try running your file with Python 2, and see what happens. You get a syntax error on some of your print lines because end=... is invalid syntax for the print statement. The easy way to fix that is:

from __future__ import print_function


When you define word_to_guess, you are really just making a list of each character in the word. There is a much simpler way:

word_to_guess = list(word_guess)


As for the currently definition, use a list comprehension:

currently = ['_' for _ in word_to_guess]


(_ is a variable name that means basically that this variable isn't used.) Since a string is immutable, you could, more simply, do this:

currently = ['_'] * len(word_to_guess)


or

currently = list('_' * len(word_to_guess))


All the code I have mentioned so far (and the rest of the code before hangman_graphic) is out in the open. You should use an if __name__ == '__main__' block to prevent that code from executing if this module is imported.

A for i in ... loop goes through ... and assigns i to each item. That doesn't need to be numbers. Instead of getting all the indexes of a list, just get the items directly:

for char in currently:
    print(char, end=" ")


or, since all the items in currently are strings, use str.join():

print(" ".join(currently))


The same goes for guess_letters.

An easier and more portable way of displaying the graphics would be a list:

graphics = [
"""
________      
|      |      
|             
|             
|             
|             """,
"""
________      
|      |      
|      0      
|             
|             
|             """,
...


Of course, you would fill out all the graphics (except the win and lose). You could then do something like:

if guesses == -1:  # Won game
    print("       0      ")
    ...
try:
    print(graphics[guesses])
except IndexError:  # Lost game
    print("________      ")
    print(...)
    ...


Global variables! Run before they kill you! Global variables are not the solution. I don't care what the problem is. If it's just one variable that you are changing, return it instead. If you absolutely, positively need global variables, use a class instead. You just don't need global variables. It gets messy when you use global variables. (Don't worry; I won't start complaining about the coming of a pony.)

You ask the user to type a single letter, but users don't always listen to good advice. You should make sure that it is a single letter. If it isn't, just tell the user the problem, and continue.

You have some nested if-else blocks. That isn't necessary. Instead, check if the letter is valid. You can have multiple elifs. That means you have if user_input in guess_letters: and else: with just one layer of nesting. The structure would be something like:

user_input = input(...).lower()
if len(user_input) != 1:
    print("Please input a single character.")
elif user_input not in accepted_input:
    print("Please input a letter in range a-z.")
elif user_input in guess_letters:
    print("You have already inputted this letter before.")
else:
    guess_letters.append(user_input)
    if user_input in word_to_guess:
        print("Well done; you guessed the right letter.")
        for i in range(len(word_to_guess)):
            if word_to_guess[i] == user_input:
                currently[i] = user_input
    else:
        print("Oops; there is no such letter in our word.")
        guesses += 1


Now doesn't that look cleaner?

Why does hangman() use guesses? We know that it should start with zero, don't we? Just use hangman_graphic(0).

Code Snippets

from string import ascii_letters # as accepted_input
from __future__ import print_function
word_to_guess = list(word_guess)
currently = ['_' for _ in word_to_guess]
currently = ['_'] * len(word_to_guess)

Context

StackExchange Code Review Q#136255, answer score: 4

Revisions (0)

No revisions yet.