10
\$\begingroup\$

I am a beginner programmer with not much experience, the little I do have is with Python specifically.

With all the fuss going around about the popular word game "Wordle" I wanted to challenge myself with the task of creating my own clone version of it. I have all of the functionality I wanted working, for example, basic input filtering.

One of the main concerns I have while programming is the fear of just writing pure unreadable spaghetti code that would never be accepted by anyone who has a lot more experience than myself.

Another one of my main concerns is basic naming conventions and basic code format. Seemingly there is no absolute correct variable naming conventions (to my knowledge) so I went with lower case function names with underscores (def word_validation) and camelCase variable names.

As for the basic layout for code I know not required for Python it is still good to have a Main method but I am not sure that I call it from the bottom of my program, where I currently am.

Basically, I am asking if anyone has any suggestions on formatting for my code, minor optimizations I can make, or telling me I wrote spaghetti and how to maybe clean it up.

GAME OF WORDLE

This game chooses a random 5 letter word from wordlib.txt and assigns it as the "Wordle".

wordlib.txt is a text file with a new 5 letter word on each line.

The player has six chances to guess the correct word.

Each round you will receive basic information about your guess.

"g" represents a letter that is correct and is also in the correct position in the word.

EXAMPLE:

Wordle: Slate
Guess: Clamp
Output: xggxx

"y" represents a correct letter but is not in the correct position in the word.

EXAMPLE:

Wordle: Slate Guess: Aspen Output: yyxyx

"x" represents an incorrect letter

EXAMPLE:

Wordle: Slate Guess: Horns Output: xxxxy

import random

ATTEMPTS = 5
attempt_number = 5
word_length = 5
is_valid = False
reset = False
guess_dict = {
    "0": "last",
    "1": "fifth",
    "2": "forth",
    "3": "third",
    "4": "seccond",
    "5": "first"
}

def random_word():
    wordle = random.choice(open("wordlib.txt").readlines())
    wordle = wordle.replace("\n","")
    print("Ssssshh.. The random word is: " + wordle + " (this is for debug purposes)")
    print("All guesses must be " + str(word_length) + " letter words")
    return wordle

def take_user_guess():
    while is_valid == False:
        guess = input("What is your " + str(guess_tracker()) + " guess: ")
        guess = guess.upper()
        if len(guess) != word_length:
            print("Sorry, your answer must be a " + str(word_length) + " letter word.")
            continue
        if not guess.isalpha():
            print("Sorry, your answer must only contain letters.") 
            continue
        if check_word(guess) == False:
            print("Sorry, your answer must be a real word.")
            continue
        event_correct_guess()
        return guess

def guess_tracker():
    return guess_dict[str(attempt_number)]

def event_correct_guess():
    global attempt_number
    if attempt_number <= 0:
        event_loss()
    else:
        attempt_number = attempt_number - 1

def check_word(guess):
    with open('wordlib.txt') as f:
        if guess not in f.read():
            return False

def eval_guess(guess, wordle):
    result = ""
    if guess != wordle:
        for i in range(word_length):
            if guess[i] == wordle[i]:
                result += "g"
            elif guess[i] in wordle:
                result += "y"
            elif guess[i] not in wordle:
                result += "x"
        print(result)
        return False
    else:
        return True

def event_win():
    print("Congrats")

def event_loss():
    print("Sorry, you lost.")
    event_replay()

def event_replay():
    global attempt_number
    playAgain = input("Do you want to play again? (Yes/No)")
    if playAgain.upper() == "YES":
        attempt_number = ATTEMPTS
        main()
    else: exit()

def main():
    global reset
    win = False
    wordle = random_word()
    while win == False:
        guess = take_user_guess()
        win = eval_guess(guess, wordle)
    event_win()
    event_replay()

if __name__ == "__main__":
    main()
\$\endgroup\$
4
  • \$\begingroup\$ The evaluation part can be written as this: ''.join('g' if a == b else 'y' if a in wordle else 'x' for a, b in zip(guess, wordle)) \$\endgroup\$ Feb 10, 2022 at 12:44
  • \$\begingroup\$ And the word_length variable is unnecessary because it is explicitly stated in the rules that the word must be exactly 5 letters long. \$\endgroup\$ Feb 10, 2022 at 12:47
  • \$\begingroup\$ micro-review: guess_dict ought to contain fourth and second rather than misspelt versions - especially in a word game! \$\endgroup\$ Feb 10, 2022 at 12:57
  • \$\begingroup\$ Your guess_dict really shouldn't be a dict, it should be a list of the six word strings, using integer string as keys for dictionary indexing is a really inefficient way, just use a flat list and use the integers themselves as indexes directly instead. Also you have many string concatenations that would be better expressed as f-strings. \$\endgroup\$ Feb 10, 2022 at 12:59

4 Answers 4

5
\$\begingroup\$

This is a solid first stab for a beginner. Here are my main points of feedback:

  1. Your script's functions call each other in a way that lead to infinite recursion for some sequences of user inputs, e.g. main() -> event_replay() -> main() -> event_replay() -> ... and main() -> take_user_guess() -> event_correct_guess() -> event_loss() -> event_replay() -> main() -> take_user_guess() -> ....

    This means that someone replaying your game indefinitely within the same running game instance would eventually be met with a program crash due to a stack overflow error, due to running out of space on the call stack.

    To fix this, you'll need to refactor your script so that you use loops instead of infinite recursion to repeat gameplay. Compare the two examples below.

    """
    An example of what NOT to do. Bad because of infinite recursion:
    main() -> game() -> replay() -> main() -> game() -> replay() -> ...
    """
    
    def game():
        # ...
        replay()
    
    def replay():
        # ask user if they want to play again
        if play_again:
            main()
    
    def main():
        game()
    
    """
    An example of what should be done instead.
    One way of structuring the game flow without infinite recursion.
    """
    
    def game():
        # This function does NOT call main() at the end.
        # When this function exits, the game is over
        # and this function's invocation is popped off the call stack.
    
    def main():
        while True:
            game()
            # ask user if they want to play again
            if not play_again:
                break
    
  2. Every call to check_word (which validates that a guess is a real 5-letter word) opens wordlib.txt, reads it all into one giant string, then does a fairly expensive check to see if guess is a substring of that giant string.

    Similarly, each call to random_word (which is called on each replay of the game) opens up the same file and picks a random word.

    We can do much better than this, since we only need to read in a list of words into memory from the file wordlib.txt once at the beginning of the script, and then we can re-use those results, now stored in memory, as many times as we want later on without constantly having to re-open that file.

    def get_words():
        with open("wordlib.txt") as f:
            return [
                word
                for line in f
                if (word := line.strip())
            ]
    
    words = get_words()
    valid_five_letter_words = set(words)
    

    Reading in the file into a list of words words: list[str], we can use this to pick a random word wordle = random.choice(words). Then we create a set from that list valid_five_letter_words: set[str] = set(words) which we can use for fast membership tests in check_word, i.e.

    def check_word(guess):
        return guess in valid_five_letter_words
    
  3. The game logic is simply incorrect, or at least inconsistent with the official Wordle game rules. In the official game, if you repeat a letter in a guess more than it appears in the target word, any excess letters will be marked as gray. Refer to the below examples:

    • If the target word is redux and my guess is daddy, the script displays yxgyx when it should be xxgxx, since there is only one occurrence of d in redux.

    • If the target word is bassy and my guess is sassy, the script displays ygggg when it should be xgggg, since there are only two occurrences of s in bassy.

    • If the target word is caret and my guess is beefy, the script displays xyyxx when it should be xyxxx, since there is only one occurrence of e in caret.

  4. You were able to make things work by using the global keyword, which isn't uncommon to see in beginner code when you're just trying to get things to work — so I wouldn't concern yourself too much about using it in this script — but using global is generally a code smell because it makes the code harder to reason about, plus you can almost always refactor your code to avoid using it, for example by using instance variables in a class or passing in the state of the game via parameters to your functions. Becoming dependent on using global will inevitably lead to "unreadable spaghetti code", something you mentioned you wanted to avoid, so I'd say it's better to get in the habit early of not using it at all, if you can avoid it.

\$\endgroup\$
4
\$\begingroup\$

The organization into functions could use some work. I could give some nits about the rest, but I think it's decent for beginner code already.

All functions starting with event_ look like they should be refactored to me.

Let's think about how a reader will approach this. A smart reader will start with main:

def main():
    global reset
    win = False
    wordle = random_word()
    while win == False:
        guess = take_user_guess()
        win = eval_guess(guess, wordle)
    event_win()
    event_replay()

Ideally, we want them to understand the whole program without having read the definitions of any of these functions. They should know which function to open up if they want to change anything.

Your main makes some sense but there seem to be missing parts for a reader. Below is a better main. It's better in that a reader can see exactly how all the parts interact. There are no global variables, and you can guess where user interactions will be printed.

def play_one_game():
    wordle = random_word()
    display_intro_text()
    for attempt_number in range(ATTEMPTS):
        guess = take_user_guess(attempt_number)
        display_guess_feedback(guess, wordle)
        won = is_guess_correct(guess, wordle)
        if won:
            display_win(wordle)
            return
    display_loss(wordle)
def main():
    keep_playing = True
    while keep_playing:
        play_one_game()
        keep_playing = ask_to_replay()
\$\endgroup\$
1
\$\begingroup\$

There are some parts of the code that are problematic:

  • Due to the guess.upper in take_user_guess, I never guess correctly
  • Because you limit the amount of possible words, I can guess weird words that you might not have thought of and still be told that it is an invalid word.
\$\endgroup\$
2
  • \$\begingroup\$ Presumably the word-list file contains a full dictionary of uppercase words? \$\endgroup\$ Feb 10, 2022 at 12:58
  • \$\begingroup\$ @Toby good point, I did not think about that. \$\endgroup\$
    – norway-yv
    Feb 11, 2022 at 6:42
0
\$\begingroup\$

There is a problem with the eval_guess function, as was previously pointed out. It returns a yellow match for any guess matching any entry. For example, if the word is "draft" and the guess is "added", the program will give a yellow match for every d in added. The official Wordle game would return only a single yellow match with the rest of the d's being gray. Here is my version:

def eval_guess(guess,target):
    dLtrs=dict() 
    output = []

    # Build dictionary for counts of target letters - it will not include instances
    #   where guess letter is correctly placed
    for ltr in (ltr for (idx,ltr) in enumerate(target) if target[idx] != guess[idx]):
            dLtrs[ltr] = 1 if ltr not in dLtrs else dLtrs[ltr] + 1

    # Look for guess matches of target letters    
    for (idx,ltr) in enumerate(guess):
        # Letter is correctly placed in guess
        if ltr == target[idx]: 
           output.append("g")
        # Guess letter is somewhere else in target
        elif ltr in dLtrs and dLtrs[ltr] > 0:   
           output.append("y")
           dLtrs[ltr] -= 1   # Decrement remaining possible dictionary letter matches
        # No match for guess letter anywhere in target
        else: 
          output.append("x") 
            
    return "".join(output)

I kind of felt uneasy about the way I used the dictionary variable. It kind of seemed like a kludge. I looked into it and found that Python has a better alternative. The collections library has a Counter class. You create an instance by giving it an iterable. Python does the work of counting how many instances there are of each item value. Accessing the Counter variable the same way as the dictionary variable returns the number of instances there are for a particular value.

For what it is worth, here is a version of eval_guess that does not use a dictionary or Counter. It loops over the target letters rather than the guess letters. This allows the program to use the output array to determine if a matching guess instance has been previously used.

def eval_guess(guess,target):
    output = ["g" if target[i]==guess[i] else "x" for i in range(5)]

    for wLetter in [letter for (k,letter) in enumerate(target) if letter != guess[k]]: 
        
        idx = next((j for (j,gLetter) in enumerate(guess) if  gLetter==wLetter\
                      and output[j]=="x"),None)
        if idx != None:
            output[idx]="o"
    return "".join(output)
\$\endgroup\$
5
  • 3
    \$\begingroup\$ Welcome to the Code Review Community. Our goal here is to help each other improve their coding skills by making insightful observations about the code, providing alternate solutions after making an observation is okay, but not necessary. Only providing alternate solutions is considered a poor answer and may be down voted or deleted by the community. This answer is okay but it could be improved by making more observations. Please read How do I write a good answer. \$\endgroup\$
    – pacmaninbw
    Feb 1 at 13:40
  • \$\begingroup\$ I am a bit confused. I explained why the solution offered was incorrect and provided a counter-example. Is that wrong? I then showed code that fixes the problem, and included comments to explain how it works. Is there a problem with that? If the code can be improved upon then people are free to show how that can be done. \$\endgroup\$ Feb 3 at 12:41
  • \$\begingroup\$ I didn't say the answer was bad I said it was okay, and I didn't down vote the answer. I',m saying there are already better answers posted. Your answers in the future could be great answers rather than Okay answers. Look at other answers and see what got the most votes. When I first joined code review several of my answers got down votes because I put too much code in without good explanations. Look at other questions as well. \$\endgroup\$
    – pacmaninbw
    Feb 3 at 14:31
  • \$\begingroup\$ I read the posts again and I saw a previous criticism of the eval_guess function, but I do not see any alternatives presented. Could you please point one out. I am submitting a cleaned up version of eval_guess \$\endgroup\$ Feb 3 at 14:56
  • \$\begingroup\$ Your code might work correctly (I did not test it) and you inserted some comments - that's okay. But, and that's a big but, especially when presenting solutions to beginners: Indenting and naming of variables do not follow PEP8. Secondly: without any explanation, list comprehensions are hard to understand for beginners. \$\endgroup\$
    – AcK
    Feb 3 at 22: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.