1
\$\begingroup\$

I have recently made a hangman game using python. This is the main bulk of the code, however, there are other files that I have writen that get the words from a dictionary, or see if a word repeats any letters.

import Pick_Word, Unique_Chars
word = (Pick_Word.get_word()).upper()
wordlist = list(word)
dict = dict(enumerate(wordlist))
slots = list('_' * len(word))
print(' '.join(slots))
def game(word):
    guess_num = 12
    already = []
    while '_' in slots:
        x = input("Guess the letter: ")
        while x in already:
            x = input("Guess the letter: ")
        for i in range(len(x)):
            if x[i] not in already:
                already.append(x[i])
        turns = True
        for a,b in dict.items():
            if x.upper() == word:
                return 'Win\nOnly {} more turns left over'.format(guess_word.title())
            elif len(x) > 1:
                single = Unique_Chars.unique(x)
                length = len(single)
                for idx in single:
                    if b == idx.upper():
                        slots[a] = idx.upper()
                        new_word = ' '.join(slots)
            elif b == x.upper():
                slots[a] = x.upper()
                new_word = ' '.join(slots)
            new_word = ' '.join(slots)
        if len(x) > 1:
            for i in range(length):
                if single[i].upper() not in word:
                    guess_num -= 1
        elif x.upper() not in word:
            guess_num -= 1
        print(new_word, '\n{} turns left'.format(guess_num))
        if guess_num == 0:
            return 'Lose\nActual Word was: {}'.format(word.title())
        elif word == str(slots):
            return 'Win\nOnly {} more turns left over'.format(guess_word.title())
print(game(word))

Is there anyway that I could shorten this code down, or make it run better, without completly rewirting the entire thing? Any feedback is much appreciated. Thanks

\$\endgroup\$
  • \$\begingroup\$ Hi, I suggest you include those missing modules Pick_Word & Unique_Chars to help people reviewing your code understand as well as provide a better more meaningful review. \$\endgroup\$ – bullseye Dec 31 '19 at 3:12
3
\$\begingroup\$
  1. Add type annotations. (This is usually my first piece of advice to all new Python coders: start using type annotations and mypy.) Adding some quick type annotations to your code and running mypy on it turned up the following bugs, which is a good demonstration of the sorts of mistakes that type checking can save you from:
hangman.py:18: error: Missing return statement
hangman.py:31: error: Name 'guess_word' is not defined
hangman.py:53: error: Name 'guess_word' is not defined
  1. You probably want to use a Set rather than a List to track things like letters that have already been guessed (since you want it to be unique and you mostly want to check to see if things are in it).

  2. Look for opportunities to create functions for discrete tasks. "Get a letter from the user" is one that jumps out at me. Creating a function that takes over the task of getting a guess and making sure it's in the expected format (i.e. a single upper-case letter) lets us remove a lot of code that has to repeatedly try to normalize that input.

  3. Having tackled that, I see that there's a lot of code that tries to handle the case of the input being the complete word -- I'm not sure if that's a coding error or just confusion about the game design, but being as you're prompting the user to enter one letter at a time, it seems like we can get rid of all of this.

  4. Your dict variable is unnecessary and badly named; you created it from an iterable but the only thing you're doing with it is converting it back to an iterable. Just leave it as an iterable. :) The wordlist is unnecessary for the same reason.

  5. Use whitespace and comments to offset individual blocks of code that do different things.

  6. Again: delete unnecessary code.

  7. From a design standpoint, I think your game function should either return a string representing the output or it should print it (and be solely in charge of the printing), not do some of the printing and return the rest for the caller to do.

  8. Declare variables within the tightest scope possible. Specifically, there's no reason to declare slots outside of the game function definition since it's only used within that function. In this particular case it makes no functional difference, but it makes the code harder to read if you have to look outside the function to figure out where/how its state was initialized, and in a case where the function might be called multiple times you'd have to worry about keeping track of how changes to those shared variables affect different calls. Better to avoid this if at all possible.

Here's the updated script after making the edits (I implemented my own stub Pick_Word for testing):

from typing import Set

class Pick_Word:
    @staticmethod
    def get_word() -> str:
        return "foobar"

def get_guess(already_guessed: Set[str]) -> str:
    """Prompt the user for a letter guess.
    Return value is guaranteed to be a single upper-case letter
    that's not in the already_guessed set."""
    while True:
        guess = input("Guess the letter: ").upper()
        if len(guess) == 1 and guess.isalpha() and guess not in already_guessed:
            return guess

def game(word: str) -> None:
    """Run the hangman game with the given word."""
    word = word.upper()

    already: Set[str] = set()
    guess_num = 12
    slots = list('_' * len(word))

    # Let the game begin!
    print(' '.join(slots))

    while '_' in slots:
        # Get the next guess.
        guess = get_guess(already)
        already.add(guess)

        # Substitute the guess into matching slots.
        for i in range(len(word)):
            if word[i] == guess:
                slots[i] = guess

        # If the guess was a miss, subtract one turn.
        if guess not in word:
            guess_num -= 1

        print(' '.join(slots), '\n{} turns left'.format(guess_num))

        if guess_num == 0:
            print('Lose\nActual Word was: {}'.format(word.title()))
            return

    print('Win\nOnly {} more turns left over'.format(guess_num))

game(Pick_Word.get_word())
| improve this answer | |
\$\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.