7
\$\begingroup\$

I spent some time making a simple Wordle clone in Python. The word bank consists of all words of a specified length taken from nltk's Brown corpus. I would appreciate any advice or criticism to improve my program's coherence and efficiency.

import random
import re
from nltk.corpus import brown
from termcolor import colored

allowed_attempts = 6
word_length = 5
word_bank = [word.upper() for word in brown.words() if len(word) == word_length and word.isalpha()]

def validate(attempt, answer, current_alpha):
    result = ''
    for pos, char in enumerate(attempt):
        if char == list(answer)[pos]: # Letter is correct and in right position
            result += (' ' + colored(char, 'green') + ' ')
            current_alpha = current_alpha.replace(char, (colored(char, 'green')))
        elif char in answer: # Letter is correct but in wrong position
            result += (' ' + colored(char, 'yellow') + ' ')
            current_alpha = current_alpha.replace(char, (colored(char, 'yellow')))
        else: # Letter is incorrect
            result += (' ' + char + ' ')
            current_alpha = current_alpha.replace(char, (colored(char, 'red')))  
    print('\n' + result + '\n')
    return current_alpha

def guess(answer):
    alphabet = ('\n\n   Q  W  E  R  T  Y  U  I  O  P   \n    A  S  D  F  G  H  J  K  L    \n      Z  X  C  V  B  N  M     \n\n')
    print('\nEnter your ' + str(allowed_attempts) + ' guesses. \n')
    for i in range(allowed_attempts):
        while True:
            print(alphabet)
            attempt = input('Attempt #' + str(i + 1) + ': ').upper()
            if len(attempt) == word_length and attempt in word_bank: # Ensures user input is valid
                break
            else:
                print(colored('\nNot in word bank.', 'red'))
        if attempt == answer: # Checks if user won
            return True
        else:
            alphabet = validate(attempt, answer, alphabet)
    return False
    
def main():
    print('\n\n\nI am thinking of a ' + str(word_length) + '-letter word. Can you guess in ' + str(allowed_attempts) + ' tries? \n')
    print('If a letter is in the right place, it will be ' + colored('green', 'green') + '.\n' +
    'If it is in the word but in the wrong place, it will be ' + colored('yellow', 'yellow') + '.\n' +
    'If it is not in the word, it will be ' + colored('red', 'red') + '.\n')
    while True:
        answer = random.choice(word_bank) # Picks a word for this turn
        if guess(answer):
            print(colored('\nCongratulations! You won!\n', 'green', attrs=['bold']))
        else:
            print('\nYou lost! The word was ' + colored(answer, 'green') + '\n')
        if input('\nWant to play again with a new word? Type anything to keep playing, or type Q to quit. ').upper() == 'Q':
            print('\nThanks for playing.\n')
            exit()

main()
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Very good code overall! Just some improvements that came to mind:

What if the data is not yet downloaded

If the corpus is not yet downloaded the code will crash, you should do instead:

import nltk
try:
    word_bank = [word.upper() for word in brown.words() if len(word) == word_length and word.isalpha()]
except RuntimeError: # data is not yet downloaded.
    nltk.download(brown)
    word_bank = [word.upper() for word in brown.words() if len(word) == word_length and word.isalpha()]

Remove repetition

In this if else chain the only thing changing is the color so:

    if char == list(answer)[pos]: # Letter is correct and in right position
        result += (' ' + colored(char, 'green') + ' ')
        current_alpha = current_alpha.replace(char, (colored(char, 'green')))
    elif char in answer: # Letter is correct but in wrong position
        result += (' ' + colored(char, 'yellow') + ' ')
        current_alpha = current_alpha.replace(char, (colored(char, 'yellow')))
    else: # Letter is incorrect
        result += (' ' + char + ' ')
        current_alpha = current_alpha.replace(char, (colored(char, 'red')))  

becomes:

    if char == list(answer)[pos]: # Letter is correct and in right position
        color = 'green'
    elif char in answer: # Letter is correct but in wrong position
        color = 'yellow'
    else: # Letter is incorrect
        color = 'default' # or white or black
    current_alpha = current_alpha.replace(char, colored(char, color))

So it is clear that the if else is only deciding the color. Even better would be to return a data structure [ (character, guessing_level), ...] for simpler testing and debugging and then go from the data structure to a pretty output.

Constant UPPERCASE

Just a useful convention in Python.

allowed_attempts = 6
word_length = 5

should be uppercase.

Small user friendlyness improvement.

    if input('\nWant to play again with a new word? Type anything to keep playing, or type Q to quit. ').upper() == 'Q':

should be:

    if input('\nWant to play again with a new word? Type anything to keep playing, or type [q]uit to quit. ').upper().startswith('Q'):

So that if the user types "quit" it also works.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you so much, this is great! No idea how I overlooked the repetition in that validation function, this is so much better. I'm going to go implement these changes now. As a relative beginner, I really appreciate this. \$\endgroup\$ Feb 1 at 16:08
6
\$\begingroup\$

Couple of more improvements you can make:


You could make word_bank into a set, for \$O(1)\$ containment testing:

WORD_BANK = {word.upper() for word in brown.words() if len(word) == word_length and word.isalpha()}

...
            if attempt in WORD_BANK:
                ...

Since the WORD_BANK only contains 5-letter words, testing the length of the word is unnecessary.

You can't use a set in random.choice, so you'd need to convert it to a list for the initial word selection, but that is once-per-puzzle instead of once-per-guess lookup. answer = random.choice(list(WORD_BANK)) Alternately, store the word bank as both a list and a set.


Ranges can start at 1, eliminating the need for + 1 calculations inside the loop. You can combined this with f'...' strings, and eliminate the explicit call to str(i + 1):

    for attempt_number in range(1, allowed_attempts + 1):
        ...
            attempt = input(f'Attempt #{attempt_number}: ').upper()

There are other places you could used f-strings, like:

    print(f'\n\n\nI am thinking of a {word_length}-letter word. Can you guess in {allowed_attempts} tries?\n')

Bugs

If the secret word is BURNT and I guess TOAST, it is going to claim the first T is in the wrong spot, and the last T is in the right spot. The real Wordle would ignore the first T because the last T is in the correct spot and there are no other T's in the answer.

Your alphabet is will get longer and longer as letters are guessed in the correct and incorrect spots. For instance, the T would become "... Q W E R [yellow]T[normal] Y U ..." after processing the first letter of the guess [T]OAST, and then the T would be again replaced when the last letter was processed TOAS[T], resulting in alphabet containing:

"... Q W E R [yellow][green]T[normal][normal] Y U ..."

Successive guesses will continue to wrap the letters with additional ANSI codes. A double letter in the answer might end up either green or yellow depending on the last match of that letter in the guess word.


Design Flaw

The method validate is both displaying output and returning a value. Functions should return a value or perform an action, not both. If you were to change the game to have GUI, you could not reuse the validate function because it was printing output to a console. You would have to rewrite it.

Returning two values is allowed. The caller can print the result to the console or display it in the GUI, as appropriate.

def validate(...):
    ...
    return current_alpha, result

def guess(...):
    ...
        ...
        alphabet, guess_result = validate(attempt, answer, alphabet)
        print(f"\n{guess_result}\n")
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for these great suggestions, I appreciate you taking the time! I hadn't even heard of f-strings before this, nor did I consider adding +1 to allowed_attempts in the loop range. I was aware of the BURNT/TOAST bug and have been struggling to find an efficient way to fix it. As for the alphabet problem, maybe it could be addressed by making the object a dictionary with the letters as keys and colours as values? Finally, I have noted the problem with the validate function and will apply your suggestion. Thank you again! \$\endgroup\$ Feb 1 at 21:22
  • 1
    \$\begingroup\$ "I was aware of the BURNT/TOAST bug" ... in which case, the code really wasn't working as intended, and therefore not on topic for Code Review. The "alphabet problem" could certainly be addressed using a dictionary mapping letters to colour; of course you'll have to solve the duplicate letter green/yellow issue first. \$\endgroup\$
    – AJNeufeld
    Feb 1 at 22:15

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.