0
\$\begingroup\$

Summary

I made a hangman game using python but I had trouble writing clean pythonic code

Questions

  1. In the code snippet I provided (Full code in the link below) I used global for: loop and points variables, I know it is bad practice to use the global. What are some ways I can prevent myself from using the global statement

  2. How would you rewrite this code using OOP or functions?

  3. I think I am developing some bad habits because of my game_loop function, a majority of the functions are calling another function but I feel like this code could be so much better. So what would you do differently in the game_loop function

  4. Are you seeing any bad practices in this code?(Not including the global statement)

Code

import copy

stick_man = [

    '''
    ________
    |       |
    |
    |
    |
    |__________
    ''',

    '''
    ________
    |       |
    |       O
    |
    |
    |__________
    ''',

    '''
    ________
    |       |
    |       O
    |       |
    |
    |__________
    ''',

    '''
    ________
    |       |
    |       O
    |       |\\
    |
    |__________
    ''',

    '''
    ________
    |       |
    |       O
    |      /|\\
    |
    |__________
    ''',

    '''
    ________
    |       |
    |       O
    |      /|\\
    |      /
    |__________
    ''',

    '''
    ________
    |       |
    |       O
    |      /|\\
    |      / \\
    |__________
    ''']


word = 'hangman'
characters = list(word)
wrong_letters = []
loop = True
points = 0

def hide_word():
    """Loops through the character list and replaces each letter with an * """
    copied_word = copy.copy(characters)

    for index, letter in enumerate(copied_word):
        copied_word[index] = '*'

    return copied_word

hidden_characters = hide_word()
guess_word = [characters, hidden_characters] # [ ['h', 'a', 'n', 'g', 'm', 'a', 'n'], ['*', '*', '*', '*', '*', '*', '*'] ]

def get_letter_matches(player_answer):
    """
        Check if player input matches any of letters in the given word
        and returns the position and duplicates of that letter
    """
    word = guess_word[0] # ['h', 'a', 'n', 'g', 'm', 'a', 'n']

    indexes = [index for index in range(len(word)) if word[index] == player_answer] # Don't know exactly what this code does
    letter_indexes = { player_answer: indexes }

    return letter_indexes # {'a': [1, 5]}

def reveal_letters(dict, player_answer):
    """
        Takes the player input and if the letter matches the word then it will
        unhide the letter in the guess_word list
    """
    hidden_word = guess_word[1]

    for index in dict[player_answer]:
        hidden_word[index] = player_answer

    return ''.join(hidden_word) # dict[player_answer] -> [1, 5]

def print_stick_board(number):
    """Print the right stick man board"""

    length = len(number)
    print(stick_man[length])

def get_incorrect_letters(player_answer):
    """Tally up all the incorrect words"""
    word = guess_word[0]

    if player_answer not in word:
        wrong_letters.append(player_answer)

    return wrong_letters

def get_point(letter):
    """Give player 100 points for getting the correct letter"""
    global points
    if letter in characters:
        points += 100
        print('Points earned: {}'.format(points))
    else:
        print('Points earned: {}'.format(points))

def is_winner(complete_word, secret_word):
    """Determine the winner and loser"""
    global loop

    if secret_word == complete_word:
        print('You Won!')
        loop = False


def game_loop():

    while loop:

        print('__________________________________________________')
        player_letter = input('Choose a letter: ')

        hidden = reveal_letters( get_letter_matches( player_letter ), player_letter )
        incorrect_letters = get_incorrect_letters( player_letter )

        print('Your word looks like this:\n{}'.format( hidden ))

        try:
            print_stick_board( incorrect_letters )
        except IndexError:
            print('You Lost!')
            break

        get_point(player_letter)
        print('You\'ve entered (wrong): {}'.format( incorrect_letters ))
        is_winner(''.join(guess_word[0]), hidden)


game_loop()

\$\endgroup\$
  • \$\begingroup\$ I updated the code! \$\endgroup\$ – Anthony Gedeon Nov 15 at 18:30
0
\$\begingroup\$

Here on Code Review, we tend to not look at off-site code. So I'll be reviewing the functions I see, and this'll become a partial answer once you paste all your code.

Globals

Generally, the way to avoid them is to simply take more arguments to your function. Any function of the form:

def get_point(letter):
    """Give player 100 points for getting the correct letter"""
    global points
    # Stuff happens...

can be refactored as:

def get_point(letter, points):
    """Give player 100 points for getting the correct letter"""
    # Stuff happens...

However, that's not the only global you're using. The other one is shown here:

    if letter in characters:

Characters is not found in the local scope, so it's a global. Judging by the name, it's probably a constant, and global constants can be perfectly fine. However, in python we tend to mark them by naming convention with ALL_CAPS_AND_SOMETIMES_UNDERSCORES. So that should be:

    if letter in CHARACTERS:

with of course the global constant renamed appropriately as well.

You'll also need to return the points value, as you want to mutate it. More on this in a sec.

Code Duplication

Is generally a bad thing. You're doing:

    if letter in CHARACTERS:
        points += 100
        print('Points earned: {}'.format(points))
    else:
        print('Points earned: {}'.format(points))

Where you probably should be doing:

    if letter in CHARACTERS:
        points += 100
    print(f'Points total: {points}')  # <-- Python 3.6+
    return points

I've also included an f-string, usable from python 3.6 onwards. They're awesome. It's basically the same as str.format(stuff), but it's shorter and more readable.

I've also clarified your message. It's not the amount of points earned, it's total points. You should also return the points value, so you can use it further. So you can now use the function from the outside like this:

points = get_point(letter, points)

And you'll update the point value outside of your function.

\$\endgroup\$
  • \$\begingroup\$ If you know the question is off-topic, can you please refrain from answering it till it fits the scope of the site? Thank you. \$\endgroup\$ – Mast Nov 13 at 16:17

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.