2
\$\begingroup\$

This Jupyter Notebook code for the Wordle game works. After a lot of fixes, I have it working as it should. The code is a jumbled mess. I am open to any suggestions and/or criticism of my code. I am here to learn and improve my code. Any help would be much appreciated.

NLTK needs to be installed to use the code.

Link to custom word file to supplement words in NLTK dictionary: custom.txt

import time
from nltk.corpus import words
from nltk.corpus import wordnet 
import random

def Occurences(s, ch):
    return([i for i, letter in enumerate(s) if letter == ch])

def printTile(tile, guess):
    for i in range(0,5):
        color = 44 - tile[i]
        print(f'\x1b[1;{color};30m {guess[i].upper()} \x1b[0m', end=' ')
        time.sleep(0.6)
    print(flush=True)
    
def printKeyboard(tile, guess, dic):
    print()
    for key in dic:
        if dic[key] != 0:
            dic[key] = -3
    for i in range(0,5):
        if dic[guess[i]] < tile[i]:
            dic[guess[i]] = tile[i]
    for key in dic:
        color = 44 - dic[key]
        print(f'\x1b[1;{color};30m {key.upper()} \x1b[0m', end=' ')
    print(flush=True)

set_of_words = set(words.words()).union(set(open('custom.txt').read().split()))
five_letter_words = [w for w in set_of_words if (len(w) == 5 and w.isalpha())]
word = (random.choice(five_letter_words)).lower()
#print(word)   # for testing
#word = 'agora'   # for testing
guesses = 0
kybd = 'abcdefghijklmnopqrstuvwxyz'
dic = {i: 47 for i in kybd}
while True:    
    guess = (input('Enter you guess:')).lower()
    while True:
        if guess not in five_letter_words or len(guess) != 5:
            guess = input('Not a 5-letter word!! Guess Again: ')
        else:
            break
    guesses += 1
    if guess == word:
        printTile([2,2,2,2,2], word)
        print('we have a winner!!!')
        break
    wd_dic = {i: 0 for i in sorted(set(word))}
    for i in wd_dic:
        wd_dic[i] = Occurences(guess,i)
    gu_dic = {i: 0 for i in sorted(set(guess))}
    for i in gu_dic:
        gu_dic[i] = Occurences(word,i)
    tile = [0,0,0,0,0]
    for i in gu_dic:
        if i in word:
            lst = gu_dic[i].copy()
            if set(wd_dic[i]) & set(lst):
                overlap = list(set(lst).intersection(set(wd_dic[i])))
                for k in overlap:
                    tile[k] = 2
                    wd_dic[i].remove(k)
                    lst.remove(k)                  
            for j in lst:
                if j in wd_dic[i] and word[j] != guess[j]:
                    tile[j] = 2
                    wd_dic[i].remove(j)
                elif word[j]==guess[j]:
                    tile[j] = 2
                    wd_dic[i].remove(j)
                else:
                    if wd_dic[i]:
                        tile[wd_dic[i][0]] = 1
                        wd_dic[i].pop(0)
                gu_dic[i].remove(j)
    printTile(tile, guess)
    printKeyboard(tile,guess, dic)
    if guesses == 6:
        print(f'You lose!! The word was:')
        printTile([2,2,2,2,2], word)
        break
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. \$\endgroup\$
    – pacmaninbw
    Nov 16, 2022 at 15:57
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. If you have a new version of the program you'd like to have reviewed, please post a new question with a link back to this one for additional context. \$\endgroup\$
    – Mast
    Nov 16, 2022 at 20:40

1 Answer 1

3
\$\begingroup\$

Is it intentional that the keyboard doesn't also show information from previous guesses?

enter image description here

In the original game, the point of the keyboard is to remind the user of all the information they have learnt so far.


Unused imports

You import wordnet, even though you never use it. You should remove unused imports.

Why do you need both words.words() and custom.txt? The only point of the nltk dependency in your code is to get a list of words. nltk is a very hefty dependency, and nltk.corpus.words is even installed in a "random" system location (home directory) totally unrelated to the project.

Your code would be more portable, and it would be easier for other people to run your code (e.g. for the sake of this code review) if you didn't have a redundant dependency like this. If you really want the nltk words especially, you could just add them all to your custom.txt file.

Naming

All function names and variable names should be snake_case according to PEP8 and Google's Python Style Guide. So Occurrences should be renamed occurrences, and printTile should be renamed print_tile, etc.

Some of your variable names could be more self-explanatory and meaningful, e.g. dic could be called letter_colors so that readers can know at a glance what it stores. wd_dic, gu_dic, tile could also do with more meaningful names.

docstrings

It would be great practice to document your functions:

def occurences(s, ch):
    r'''Find all indexes where the character appears in the string.

    >>> occurrences('aisle', 'a')
    [0]
    >>> occurrences('ababa', 'b')
    [1, 3]
    '''
    return [i for i, letter in enumerate(s) if letter == ch]

The lines of the docstring starting >>> are called doctests, and are a great way to provide examples in your documentation at the same time as adding tests-for-correctness. Afterwards, run python -m doctest your_code.py to see if the doctests actually do have the expected results.

Type Hints

You can also use type hints to self-document your code's intended usage.

def occurences(s: str, ch: str) -> list[int]:
    ...

Unnecessarily verbose

In lines 49 and 52, you have

wd_dic = {i: 0 for i in sorted(set(word))}
...
gu_dic = {i: 0 for i in sorted(set(guess))}

Why are you converting word to a set and sorting it before looping it to make dictionary keys? You don't depend on the order of the dictionary keys at any point.

wd_dic = {i: 0 for i in word}
...
gu_dic = {i: 0 for i in guess}

Is logically equivalent. Immediately after each, you even loop the keys to generate the values:

wd_dic = {i: 0 for i in sorted(set(word))}
for i in wd_dic:
    wd_dic[i] = Occurences(guess,i)

Just use your dict comprehension to generate the values in the first place:

wd_dic = { letter: occurrences(guess, letter) for letter in word }

The line 39 while True: loop also exhibits logical redundancy; just lift the condition of the if statement to get a while loop with a single-line loop body.

Too much verbosity obscures the purpose of the code.

Solution design

Nice to have top-level functions first and procedural programming beneath.

Top-level items, like those functions, should be separated by two blank lines instead of just one. See the PEP.

Start your procedural part with a if __name__ == '__main__' check. Then other developers can include your code as a module, and use its top-level functions, without the import process kickstarting your game.

You should notice that you only need one of wd_dic and gu_dic. Because they each individually contain complete information about how the letters of the guess are present in the letters of the word, one alone should be enough to calculate all the green, yellow, and blue tiles.

I would delegate the functionality of figuring out correct letters to its own function, instead of spreading it between lines 49-76 in the middle of the game loop logic.

def answer(word: str, guess: str) -> list[int]:
    r'''Return the correctness of each guessed letter, the "tiles".

    :returns: a mapping of the letters in guess to 0 if the letter is not
        in the word, 2 if it is in the word and in the right place, 1 otherwise

    >>> answer('lemon', 'logic')
    [2, 1, 0, 0, 0]
    '''
    places = { letter: occurrences(word, letter) for letter in guess }
    result = []
    for i, letter in enumerate(guess):
       if not places[letter]: # no occurrences
           result.append(0)
       elif i in places[letter]:
           result.append(2)
       else:
           result.append(1)
    return result

I would probably not use a dictionary at all tbh:

return [0 if letter not in word
   else 1 if word[i] != letter
   else 2
   for i, letter in enumerate(guess)]

It was moving the functionality of getting the tiles to its own function that allowed me to see this possible simplification.

Functional programming

You may want to experiment with making functionally-pure functions instead of I/O modifying ones directly.

So instead of

def print_tile(tile, guess):
    for i in range(5):
        print(guess[i].upper(), end=' ')
    print(flush=True)

...

print_tile(tile, guess)

You would have

def format_tiles(tiles, guess):
    return ' '.join(guess[i].upper() for i in range(5))
    
...

print(format_tiles(tiles, guess), flush=True)

The logic appears in pure functions, and the I/O appears in its own section and it just calls those functions.

Magic number

In a few places you've written range(0,5), len(guess) != 5 etc.

You're using 5 as a magic number, which is okay in this case since the code is for wordle specifically, and everyone understands it's a 5-letter-word guessing game, but magic numbers are bad practice in general.

Define it in a variable word_length = 5, and use this everywhere else. Your input lists of words do not only contain length-5 words anyway, so changing word_length would let you play wordle in your program with other lengths of words, which could be fun.

Functions, to remain pure, should take the word length as an input rather than using the global one, for example:

def print_tile(tile, guess):
    for i in range(len(guess)):
        print(guess[i].upper(), end=' ')
    print(flush=True)
\$\endgroup\$
4
  • \$\begingroup\$ theonlygusti, Thanks for your input. I will work on the keyboard issue as soon as I have time. It is finals weeks, so I will be busy this week. But thanks a million for the feedback!!! Ashish \$\endgroup\$
    – Ashish
    Nov 15, 2022 at 16:00
  • \$\begingroup\$ I fixed the keyboard issue in the game. Now it shows the previous guesses. I will look into all your other suggestions soon. THANKS so much for your input and time. \$\endgroup\$
    – Ashish
    Nov 16, 2022 at 15:43
  • \$\begingroup\$ theonlygusti, how do you write a pure function (print_tile) when there is internal logic and the time pause? \$\endgroup\$
    – Ashish
    Nov 16, 2022 at 19:20
  • 1
    \$\begingroup\$ @Ashish just take those out of the function. time.sleep would probably also only be part of some interactive sequence. To rewrite print_tile as a pure function, it can no longer be a function that prints a tile and returns None. Instead you should make a string that looks like the tile and return it from the function (a better name might be make_tile now). Then instead of calling print_tile() to print tiles, you have to do it yourself: print(make_tile()) \$\endgroup\$ Nov 16, 2022 at 19:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.