5
\$\begingroup\$

I got a ton of helpful tips last time I posted some code, so I thought I would come back to the well.

My function is deciding whether or not the secretWord has been guessed in my hangman game. I am trying to accomplish this by taking a list, lettersGuessed, and comparing it to the char in each index of the string secretWord. My code works, but I feel as though it is not optimal nor formatted well.

def isWordGuessed(secretWord, lettersGuessed):
    '''
    secretWord: string, the word the user is guessing
    lettersGuessed: list, what letters have been guessed so far
    returns: boolean, True if all the letters of secretWord are in lettersGuessed;
      False otherwise
    '''
    chars = len(secretWord)
    place = 0
    while place < chars:
        if secretWord[0] in lettersGuessed:
            place += 1
            return isWordGuessed(secretWord[1:], lettersGuessed)
        else:
            return False
    return True

I tried to use a for loop (for i in secretWord:) originally, but I could not get my code to return both True and False. It would only do one or the other, which is how I ended up with the while loop. It seems that while loops are discouraged/looked at as not very useful. Is this correct?

Also, I am wondering if the recursive call is a good way of accomplishing the task.

\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

Because we always return from the inside of the while, we'll never perform the loop more than once. Thus, your while actually acts like an if. Thus the value of place is not really used and you can get rid of it : if place < chars becomes if 0 < chars which is if chars which is if len(secretWord) which is really if secretWord because of the way Python considers the empty string as a false value. Thus, you code could be :

def isWordGuessed(secretWord, lettersGuessed):
    if secretWord:
        if secretWord[0] in lettersGuessed:
            return isWordGuessed(secretWord[1:], lettersGuessed)
        else:
            return False
    return True

Which can be re-written as :

def isWordGuessed(secretWord, lettersGuessed):
    if secretWord:
        return secretWord[0] in lettersGuessed and isWordGuessed(secretWord[1:], lettersGuessed)
    return True

which is then nothing but :

def isWordGuessed(secretWord, lettersGuessed):
    return not secretWord or secretWord[0] in lettersGuessed and isWordGuessed(secretWord[1:], lettersGuessed)

Now, if I was to write the same function, because it is so easy to iterate on strings in Python, I'd probably avoid the recursion and so something like :

def isWordGuessed(secretWord, lettersGuessed):
    for letter in secretWord:
        if letter not in lettersGuessed:
            return False
    return True

Then, there might be a way to make this a one-liner but I find this to be pretty easy to understand and pretty efficient.

\$\endgroup\$
1
  • \$\begingroup\$ If you wanted a one-liner for your final code, you could do return all(letter in lettersGuessed for letter in secretWord); the Python default built-ins all() and any() are good tricks to have up your sleave for crafting concise Python code. I actually think it's equally clear too, so (IMO) it's even better than your current final version. \$\endgroup\$
    – Graham
    Nov 18, 2018 at 0:15
7
\$\begingroup\$

You could think in sets of letters:

def is_word_guessed(secret_word, letters_guessed):

    return set(secret_word) <= set(letters_guessed)

Edit: to improve the answer as codesparkle suggested:

The main benefit is the clarity of expression - the reader of the code doesn't need to run a double loop, or a recursive function in his/her head to understand it.

The function and parameter names were changed in order to comply with the Style Guide for Python Code.

\$\endgroup\$
3
  • \$\begingroup\$ Interesting answer! Could you explain the benefits of using sets and maybe explain your renaming of the method to conform to python naming conventions? Thanks. \$\endgroup\$
    – Adam
    Mar 8, 2013 at 21:37
  • \$\begingroup\$ @pjz I don't think == works either because that would only tell you if the two sets were identical (i.e., the guessed letters exactly matched the secret letters and had no additional extra letters). Personally my first thought was to use sets, but I'd probably use the built in functions, namely issubset. This would then be set(secret_word).issubset(set(letters_guessed) which would tell you if all the letters in your secret word are contained in your guessed letters. The built in functions should be more clear than <= about their purpose. \$\endgroup\$
    – zephyr
    Aug 11, 2021 at 18:59
  • 1
    \$\begingroup\$ @zephyr I completely misread the code. You're completely correct. That said, I think the all(letter in letters_guessed for letter in secret_word) is the most readable solution. \$\endgroup\$
    – pjz
    Aug 12, 2021 at 20:25

Your Answer

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

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