18
\$\begingroup\$

I'm starting out with learning how to program in Python and created a Hangman game. I think there is a lot of unnecessary code but I am not sure how to improve it. I wanted to avoid using global variables but was not sure how to save the random word (as every time I called it in another function, it would be a different word).

import random

# Global Variables
d = {'barnyard':'place', 'airplane':'transportation', 'daughter':'loved     ones',
 'vacuum':'object', 'elevator':'transportation', 'football':'sport', 'swimming':'sport',
 'lemonade':'food and drink', 'handsome':'appearance', 'racoon':'animal'}
randomWord = ''
randomWordGenre = ''
matchedChar = []
displaySpaces = []
characterList = []

def main():
    while True:
        playAgain = ('y')
        while playAgain == 'y':
            gameLayout()
            resetGame()

            # Get random word, genre and split word into list.
            getRandomWord()
            getRandomWordGenre()
            getCharacterList()

            # Prompt user for level.
            gameMode = input('Choose a level.')
            if gameMode == 'easy':
                numberOfGuesses = 10
            elif gameMode == 'medium':
                numberOfGuesses = 8
            elif gameMode == 'hard':
                numberOfGuesses = 7

            # Display blank spaces and genre of word.
            print('Genre: ' + randomWordGenre)
            displayBlankSpaces()
            joinDisplaySpaces()

            # Run main game loop.
            guesses = 0
            while guesses < numberOfGuesses and checkChar() == False:
                guess = input('Enter a character. You have ' + str(numberOfGuesses - guesses) + ' guesses left.')
                matchGuess(guess)
                addGuessedCharacters()
                joinDisplaySpaces()
                if checkChar() == True:
                    print('Congratulations. You just won Hangman!')
                elif matchGuess(guess) == False:
                    guesses += 1
            if guesses == numberOfGuesses:
                print('Sorry, you lost.')
            playAgain = input('Would you like to play again? (y/n)')
        break

# Game Layout
def gameLayout():
    print ('++++++++++++++++++Hangman++++++++++++++++++++++')
    print ('===============================================')
    print ('|--  Level  --|--  Tries  --|-- Hints Given --|')
    print ('|--  Easy   --|--   10    --|--      2      --|')
    print ('|--  Medium --|--    8    --|--      1      --|')
    print ('|--  Hard   --|--    7    --|--      0      --|')

# Reset game processes.
def resetGame():
    global randomWord, randomWordGenre, matchedChar, displaySpaces, numberOfCharacters, characterList
    randomWord = ''
    randomWordGenre = ''
    matchedChar = []
    displaySpaces = []
    numberOfCharacters = 0
    characterList = []

# Get a random word from dictionary and store in global
# variable getRandomWord[]
def getRandomWord():
    global randomWord
    randomWord = random.choice(list(d.keys()))

# Get genre from chose random word and store in global
# variable randomWordGenre[]
def getRandomWordGenre():
    global randomWordGenre
    randomWordGenre = d[randomWord]

# Split string into character list and store in global
# variable characterList[]
def getCharacterList():
    for char in range(0, len(randomWord)):
        characterList.append(randomWord[char])

# Check if user guess matches any character(s) of word and saves
# index position(s) to global variable matchedChar[]
def matchGuess(guess):
    if guess in characterList:
        index = 0
        while index < len(randomWord):
            index = randomWord.find(guess, index)
            if index == -1:
                break
            matchedChar.append(index)
            index += 1
    else:
        return False

# Display blank spaces for # of characters in word and stores in
# global variable displaySpaces[]
def displayBlankSpaces():
    for x in range (0, len(characterList)):
        displaySpaces.append("_")
    return displaySpaces

# Add guessed characters to global variable displaySpaces[]
def addGuessedCharacters():
    for x in range (0, len(matchedChar)):
        displaySpaces[matchedChar[x]] = randomWord[matchedChar[x]]

# Check if all guessed characters match word and return True or False.
def checkChar():
    str1 = ''.join(str(e) for e in displaySpaces)
    return str1 == randomWord

def joinDisplaySpaces():
    str1 = ' '.join(str(e) for e in displaySpaces)
    print (str1)

main()
\$\endgroup\$
25
\$\begingroup\$

Some initial comments:

  • I started by running the game without reading the code, to see what would happen.

    $ python hangman.py
    ++++++++++++++++++Hangman++++++++++++++++++++++
    ===============================================
    |--  Level  --|--  Tries  --|-- Hints Given --|
    |--  Easy   --|--   10    --|--      2      --|
    |--  Medium --|--    8    --|--      1      --|
    |--  Hard   --|--    7    --|--      0      --|
    Choose a level.Easy
    Traceback (most recent call last):
      File "hangman.py", line 127, in 
        main()
      File "hangman.py", line 26, in main
        gameMode = input('Choose a level.')
      File "", line 1, in 
    NameError: name 'Easy' is not defined

    Two comments:

    1. It would be nice if my input was on a new line, not shoved against the "Choose a level" message.
    2. Like a lot of people, my system still defaults to using Python 2, but I can tell from this error message that you’re using Python 3. It would be nice if you’d mentioned that in the tags/question body.

  • Okay, let’s try again with Python 3:

    $ python3 hangman.py
    ++++++++++++++++++Hangman++++++++++++++++++++++
    ===============================================
    |--  Level  --|--  Tries  --|-- Hints Given --|
    |--  Easy   --|--   10    --|--      2      --|
    |--  Medium --|--    8    --|--      1      --|
    |--  Hard   --|--    7    --|--      0      --|
    Choose a level.Easy
    Genre: appearance
    _ _ _ _ _ _ _ _
    
    Traceback (most recent call last):
      File "hangman.py", line 127, in <module>
        main()
      File "hangman.py", line 41, in main
        while guesses < numberOfGuesses and checkChar() == False:
    UnboundLocalError: local variable 'numberOfGuesses' referenced before assignment

    I’ve entered the level in the same way it was listed in the table, but got another ugly traceback. The problem is this block of code here:

    if gameMode == 'easy':
        numberOfGuesses = 10
    elif gameMode == 'medium':
        numberOfGuesses = 8
    elif gameMode == 'hard':
        numberOfGuesses = 7
    

    I have to enter the level name exactly as you’ve described it, or numberOfGuesses never gets defined and it all falls over.

    At a minimum, there should be an else: block which catches invalid input, and asks the user for something sensible. Better would be if you use gameMode.lower(), or modify the strings in the table to match.

  • Now I know how to select a level, I can start playing the game. Let’s see what happens:

    Genre: animal
    _ _ _ _ _ _
    Enter a character. You have 10 guesses left.A
    _ _ _ _ _ _
    Enter a character. You have 9 guesses left.A
    _ _ _ _ _ _
    Enter a character. You have 8 guesses left.A
    _ _ _ _ _ _

    Hmm. Entering the letter “A” repeatedly seems to keep decrementing my guesses. That’s a bit sad.

    In a similar vein, I can repeatedly hit return, not entering any letters at all, and the game likes that just fine.

    In pen-and-paper Hangman, I can easily see letters which have already been guessed and rejected, and I can’t guess the same letter twice (unless the person running the game is feeling mean). It would be nice if your program did the same.

  • Oh well, I lost the game. I’d like to play again:

    Sorry, you lost.
    Would you like to play again? (y/n)Y
    
    (tmp) alexwlchan in ~/tmp
    $

    That’s not quite as egregious, because I didn’t enter exactly what was asked. Nonetheless, it’s common to do something like

    if response.lower() in ['y', 'yes']:
    

    which is a bit more friendly to human input, without being any more complicated.

    Separately, it’s often common in that sort of prompt to highlight the default with uppercase:

    Would you like to play again? (y/N)
    

That’s enough poking the game to see what happens; let’s dive into the code and see what we find.

  • You should read PEP 8, the Python style guide – there isn’t much that’s wrong here, but a few things could be tidied up. (Look particularly at the naming conventions.)

    Then read PEP 257, which explains how to use docstrings. That’s the preferred way to document functions in Python, not block comments about the definition.

  • Passing around all these global variables is a distinct antipattern, so well done for flagging it up. In general, when you have a lot of shared state, it’s better to put it in a class and have the shared data be instance attributes.

  • Since the range() function implicitly starts at 0, it’s better to drop that argument to cut down on visual noise. That said, in a lot of places where you’re using range(), there are more Pythonic approaches. A few examples:

    • In getCharacterList(), you can just do

      characterList = list(randomWord)
      

    list() will break a string into a list of one-character strings.

    • In displayBlankSpaces(), you can do

      return ['_'] * len(characterList)
      
  • Rather than putting the main() function at the top level, a common construction is to do:

    if __name__ == '__main__':
        main()
    

    That means that main() is only called if you run the script directly, but you can import this file without it being called. That makes it easier to share code.

\$\endgroup\$
1
\$\begingroup\$

You're running into the problem people often have with global variables, where you don't realise the usefulness of passing parameters and returning values from a function. You can have a function return a value when it's done, and that way you can pass a result out of a function. Likewise, you can pass variables to a function as parameters as a way to make sure they have a specific value without using global. I'll show you how the initialisation would work with this:

For getRandomWord you shouldn't set a global value, instead return the result of the random.choice:

def getRandomWord():
    return random.choice(list(d.keys()))

Now, when you call getRandomWord you can actually assign the result to the name randomWord:

randomWord = getRandomWord()

Now you can pass this into getRandomWordGenre and again return the result of that:

def getRandomWordGenre(randomWord):
    return d[randomWord]

This would be called like this:

randomWord = getRandomWord()
randomWordGenre = getRandomWordGenre(randomWord)

Note that of course you could name the parameter just word, it doesn't need to match the exact name of what you're passing.

Then you can do the same with getCharacterList, all you really need to do is create a list within the function and then return the list you've created, like so:

def getCharacterList(randomWord):
    characterList = []
    for char in range(0, len(randomWord)):
        characterList.append(randomWord[char])
    return characterList

And call it the same as before:

randomWord = getRandomWord()
randomWordGenre = getRandomWordGenre(randomWord)
characterList = getCharacterList(randomWord)

Although, as a shortcut you can pass a string to list() and get a list of the characters, so you don't need this function and could replace it with this:

characterList = list(randomWord)
\$\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.