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
-
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– pacmaninbw ♦2022年11月16日 15:57:48 +00:00Commented 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– Mast ♦2022年11月16日 20:40:35 +00:00Commented Nov 16, 2022 at 20:40
1 Answer 1
Is it intentional that the keyboard doesn't also show information from previous guesses?
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)
-
\$\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– Ashish2022年11月15日 16:00:08 +00:00Commented 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– Ashish2022年11月16日 15:43:14 +00:00Commented 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– Ashish2022年11月16日 19:20:29 +00:00Commented 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 rewriteprint_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 bemake_tile
now). Then instead of callingprint_tile()
to print tiles, you have to do it yourself:print(make_tile())
\$\endgroup\$user98809– user988092022年11月16日 19:24:55 +00:00Commented Nov 16, 2022 at 19:24