7
\$\begingroup\$

To learn Python, I've been working through Learn Python the Hard Way, and to exercise my Python skills I wrote a little Python Hangman game (PyHangman - creative, right?):

#! /usr/bin/env python2.7

import sys, os, random

if sys.version_info.major != 2:
    raw_input('This program requires Python 2.x to run.')
    sys.exit(0)


class Gallows(object):
    def __init__(self):
        '''Visual of the game.'''
        self.state = [
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t        | ',
                '\t        | ',
                '\t        | ',
                '\t        | ',
                '\t________|_',
            ],
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t  O     | ',
                '\t        | ',
                '\t        | ',
                '\t        | ',
                '\t________|_',
            ],
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t  O     | ',
                '\t  |     | ',
                '\t  |     | ',
                '\t        | ',
                '\t________|_',
            ],
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t  O     | ',
                '\t \|     | ',
                '\t  |     | ',
                '\t        | ',
                '\t________|_',
            ],
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t  O     | ',
                '\t \|/    | ',
                '\t  |     | ',
                '\t        | ',
                '\t________|_',
            ],
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t  O     | ',
                '\t \|/    | ',
                '\t  |     | ',
                '\t /      | ',
                '\t________|_',
            ],
            [
                '\t  _______ ',
                '\t  |     | ',
                '\t  O     | ',
                '\t \|/    | ',
                '\t  |     | ',
                '\t / \\    | ',
                '\t________|_',
            ]
        ]

    def set_state(self, misses):
        '''Sets the current visual being used.'''
        image = ''

        state = self.state[misses] # set state to the list of desired gallows image
        # construct gallows image into str from list
        for piece in state:
            image += piece + '\n'
        return image


class Wordlist(object):
    def __init__(self):
        '''Set the length of the wordlist.'''
        self.numLines = sum(1 for line in open('test.txt'))

    def new_word(self):
        '''Choose a new word to be guessed.'''
        stopNum = random.randint(0, self.numLines-1) # establish random number to be picked from list
        # extract word from file
        with open('test.txt') as file:
            for x, line in enumerate(file):
                if x == stopNum:
                    word = line.lower().strip() # remove endline characters
        return word


def set_blanks(word):
    '''Create blanks for each letter in the word.'''
    blanks = []

    for letter in word:
        # Don't hide hyphens
        if letter == '-':
            blanks += '-'
        else:
            blanks += '_'
    return blanks


def check_letter(word, guess, blanks, used, missed):
    '''Check if guessed letter is in the word.'''
    newWord = word

    # If the user presses enter without entering a letter
    if guess.isalpha() == False:
        raw_input("You have to guess a letter, silly!")

    # If the user inputs multiple letters at once
    elif len(list(guess)) > 1:
        raw_input("You can't guess more than one letter at a time, silly!")

    # If the user inputs a letter they've already used
    elif guess in used:
        raw_input("You already tried that letter, silly!")

    # replace the corresponding blank for each instance of guess in the word
    elif guess in word:
        for x in range(0, word.count(guess)):
            blanks[newWord.find(guess)] = guess
            newWord = newWord.replace(guess, '-', 1) # replace already checked letters with dashes
        used += guess # add the guess to the used letter list

    #If the guess is wrong
    else:
        missed = True
        used += guess
    return blanks, used, missed


def new_page():
    '''Clears the window.'''
    os.system('cls' if os.name == 'nt' else 'clear')


def reset(image, wordlist):
    wrongGuesses = 0 # number of incorrect guesses
    currentImg = image.set_state(wrongGuesses) # current state of the gallows
    word = wordlist.new_word() # word to be guessed
    blanks = set_blanks(word) # blanks which hide each letter of the word until guessed
    used = [] # list of used letters
    return wrongGuesses, currentImg, word, blanks, used


def play(image, currentImg, wrongGuesses, word, blanks, used):
    missed = False

    new_page()
    print currentImg
    for x in blanks:
        print x,
    print '\n'
    for x in used:
        print x,
    print '\n'

    guess = raw_input("Guess a letter: ")
    blanks, used, missed = check_letter(word, guess.lower(), blanks, used, missed)

    if missed == True and wrongGuesses < 6:
        wrongGuesses += 1
        currentImg = image.set_state(wrongGuesses)
        play(image, currentImg, wrongGuesses, word, blanks, used)
    elif missed == False and blanks != list(word) and wrongGuesses != 7:
        play(image, currentImg, wrongGuesses, word, blanks, used)
    elif blanks == list(word):
        endgame('win', word)
    else:
        endgame('lose', word)


def endgame(result, word):
    new_page()
    if result != 'lose':
        print "Congratulations, you win!"
        print "You correctly guessed the word '%s'!" % word
    else:
        print "Nice try! Your word was '%s'." % word
    while True:
        play_again = raw_input("Play again? [y/n]")
        if 'y' in play_again.lower():
            return
        elif 'n' in play_again.lower():
            sys.exit(0)
        else:
            print "Huh?"


def main():
    image = Gallows()
    wordlist = Wordlist()

    new_page()
    print("\nWelcome to Hangman!")
    print("Guess the word before the man is hung and you win!")
    raw_input("\n\t---Enter to Continue---\n")
    new_page()

    while True:
        misses, currentImg, word, blanks, used = reset(image, wordlist)
        play(image, currentImg, misses, word, blanks, used)

if __name__ == '__main__':
    main()

So what do you all think? This is the second iteration of the program, as the first one was a mess, but I think I've got it all worked out a bit better now.

One of the things that I still think is messy is how I had the game loop after you guessed a letter, as it simply calls itself again. Is there a better way to do that?

Any tips on making my code more pythonic is appreciated!

\$\endgroup\$
  • \$\begingroup\$ A bit of a naive question why is '\' escaped in the right leg but not in the left arm ? \$\endgroup\$ – Josay Mar 3 '14 at 17:14
  • \$\begingroup\$ Hm, good question. I'm not sure why I originally did that, but removing it doesn't seem to break anything. \$\endgroup\$ – DaimyoKirby Mar 3 '14 at 17:45
  • \$\begingroup\$ Here's a followup question. \$\endgroup\$ – DaimyoKirby Apr 15 '14 at 16:56
10
\$\begingroup\$

I'll get a few trivial comments out of the way before addressing some serious concerns about the flow of control in your program.

User experience

Overall, the application feels like it's nicely put together. I enjoyed playing it while writing this review.

  • The past participle of the verb "to hang" is "hung", but when talking about executions, it should be "hanged". ("Hung" isn't completely unacceptable usage, though.)
  • endgame() should not clear the screen. For me, part of the fun of playing Hangman is to see the final state of the game, and clearing the screen denies me that pleasure.

Python 3 compatibility

You explicitly check that the game is running on Python 2. With a few changes, all related to input() and print(), you can also get it to run on Python 3.

  • Put the following shim at the top, then replace all raw_input() calls with input().

    if sys.version_info.major < 3:
        # Compatibility shim
        input = raw_input
    
  • Replace all print "string" with print("string")

  • However, the following printing code, which uses a trailing comma to suppress the newline in Python 2, requires special treatment:

    for x in blanks:
        print x,
    print '\n'
    for x in used:
        print x,
    print '\n'
    

    I suggest the following replacement code, which takes a better approach even if you weren't interested in Python 3:

    print(' '.join(blanks))
    print(' '.join(used))
    

Wordlist

  • Buried in the middle of a long program, you hard-coded "test.txt". That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().

Gallows

  • Gallows.set_state() is a misnomer. The Gallows object doesn't actually keep any state! A more accurate name would be Gallows.get_image().
  • I would prefer it if Gallows did keep track of the number of wrong guesses, and had methods .increment_count() and .is_hanged(). Currently, you hard-code 6 as a limit in play(), a number that you hope is consistent with the number of images available in Gallows (minus one for the initial image).

Interaction between reset(), check_letter(), and play()

  • You pass clusters of variables around a lot. Those are actually state variables, so these three functions would be better as members of a class.
  • check_letter() should not take missed as a parameter. It's an output, not an input.
  • The way check_letter() handles correct guesses is clumsy. I would write

    # replace the corresponding blank for each instance of guess in the word
    elif guess in word:
        for index, char in enumerate(word):
            if char == guess:
                blanks[index] = guess
        used += guess # add the guess to the used letter list
    

Flow of control

All the critiques above are minor details. The most important problem with the code, in my opinion, is that you are misusing recursive function calls as a kind of goto.

To see a symptom of the problem, play a few rounds, then hit CtrlC. Notice that the stack trace is very deep — it contains one call to play() for every guess ever made in the history of the program, including previous rounds. It is inappropriate to keep such state around in the call stack.

The remedy is to use loops for looping.

Here's how I would write it (without making too many of the recommended changes listed above).

def play(word):
    """ Play one game of Hangman for the given word.  Returns True if the
        player wins, False if the player loses. """
    gallows = Gallows()
    wrong_guesses = 0                           # number of incorrect guesses
    blanks = set_blanks(word)                   # blanks which hide each letter of the word until guessed
    used = []                                   # list of used letters

    while True:
        new_page()
        print(gallows.get_image(wrong_guesses))
        print(' '.join(blanks))
        print(' '.join(used))

        guess = input("Guess a letter: ")
        blanks, used, missed = check_letter(word, guess.lower(), blanks, used)

        if blanks == list(word):
            return endgame(True, word)
        elif missed and wrong_guesses >= 6:
            return endgame(False, word)
        elif missed:
            wrong_guesses += 1

def endgame(won, word):
    print('')
    if won:
        print("Congratulations, you win!")
        print("You correctly guessed the word '%s'!" % word)
    else:
        print("Nice try! Your word was '%s'." % word)
    return won

def play_again():
    while True:
        play_again = input("Play again? [y/n] ")
        if 'y' in play_again.lower():
            return True
        elif 'n' in play_again.lower():
            return False
        else:
            print("Huh?")


def main(words_file='test.txt'):
    wordlist = Wordlist(words_file)

    new_page()
    print("\nWelcome to Hangman!")
    print("Guess the word before the man is hanged and you win!")
    input("\n\t---Enter to Continue---\n")
    new_page()

    while True:
        play(wordlist.new_word())
        if not play_again():
            break
\$\endgroup\$
4
\$\begingroup\$

Python supports multi-line string literals. You could use those instead of using a list to encode your state image.

a = r""" 'hello' \n
  more """
print a

You have multiple methods called set_XXX() that return values instead of storing a value. The names of these methods could be changed to better describe what they are doing.


Wordlist.new_word() will continue to iterate over the entire file even after the selected word is found. Putting the return where you create the word variable will break the loop once you have the value you want.


Wordlist uses a hard coded string for the file name, that is specified in two different places. If you want to change to a different file, you have to remember to change both cases. This is a simple example, that may seem trivial, but this type of error is hard to track down when the code becomes larger and more complicated.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.