2
\$\begingroup\$

Made after seeing A Simple Word Guessing Game. Any code critique is welcome!

import urllib.request as Web
import random
import string


def get_words():
    url = "http://svnweb.freebsd.org/csrg/share/dict/words?view=co&content-type=text/plain"
    req = Web.Request(url)
    res = Web.urlopen(req).read().splitlines()
    return res


def get_target():
    words = get_words()

    while True:
        target = random.choice(words).decode('utf8')

        if len(target) > 2 and "'" not in target and '.' not in target:
            return target


def get_guess():
    print('Guess a single letter!')

    while True:
        guess = input().strip()

        if len(guess) == 1:
            if guess.isalpha():
                return guess
            elif guess.isdigit():
                return string.ascii_lowercase[int(guess)]
            else:
                print('[!] Needs to be alnum << ', end='', flush=True)
        else:
            print('[!] Needs only one char << ', end='', flush=True)


def play_game():
    print('Fetching a random word...')
    target = get_target()
    comp = ['_' for _ in target]
    print("Alright, we're ready to play!\n")

    while True:
        print('Target:', ' '.join(comp))
        guess = get_guess()

        if guess in target:
            for i, c in enumerate(target):
                if c == guess:
                    comp[i] = c

        if '_' in comp:
            print()
        else:
            print('Congrats! You won!')
            return 0


if __name__ == "__main__":
    play_game()

\$\endgroup\$
2
\$\begingroup\$
  • I think you should filter out the words you don't want in your game (words with less than 3 characters) in get_words. That way it's convenient for you if you need to pick from the word list multiple times. For example, you might want to give the player the option to 'play again' with a different word. If you do that (and adopt my suggestion about words with punctuation below), you won't need get_target anymore and can just call random.choice directly on the list of words.

  • Consider using requests to simplify your code:

    words = requests.get(url).text.splitlines() # List[str]
    
  • For easier testing, it would be more convenient if play_game took in target: str as a parameter. This way you can do ad-hoc testing with a word of your choice.

  • When you iterate over target to generate comp, you can also construct a mapping of letters to indices in target. This makes revealing correctly guessed characters in target more efficient:

    """
    Note that this also handles words with punctuation
    by filling them in for the player, e.g.
    
       R&D -> _ & _
      Ph.D -> _ _ . _
    you'll -> _ _ _ ' _ _
    """
    char_to_indices = defaultdict(list)
    comp = []
    for i, c in enumerate(target):
        if c in string.ascii_letters:
            char_to_indices[c.lower()].append(i)
            comp.append('_')
        else:
            comp.append(c)
    

    Revealing instances of the correctly guessed character:

    if guess in char_to_indices:
        for i in char_to_indices[guess]:
            comp[i] = target[i]
    

General comments about usability

  • It's a fairly large word list, so consider caching it to a file so the player doesn't have to download it again each time they want to play.

  • If the secret word is 'Apple' and the player guesses 'a', the game doesn't fill in 'A'. I think it would be a better play experience if the game wasn't strict about the case of the guessed letter (this is handled above in the code snippets with char_to_indices).

  • When searching the word list, I didn't see any entries that had numerical digits 0-9. Interpreting a digit character as an index into the string string.ascii_lowercase is unintuitive/surprising behavior as well. Seems like get_guess should only complain if the player didn't enter exactly one character from the alphabet.

  • One improvement to be more user-friendly might be to print out a reference 'letter bank' of all the alphabet letters the player hasn't guessed yet:

    # start with all 26 letters
    characters_left = string.ascii_lowercase
    
    # printing out the 'letter bank'
    print('Target:', ' '.join(comp))
    print(f'\n[ {characters_left} ]')
    # ...
    
    # removing a player's guess
    characters_left = characters_left.replace(guess, ' ')
    
  • When the player wins, print out the entire word! Currently the game just ends abruptly with the congratulatory message.

\$\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.