3
\$\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\$
5
\$\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\$
  • \$\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 '18 at 0:15
6
\$\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\$
  • \$\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 '13 at 21:37
  • \$\begingroup\$ Note that your current test "<=" is incorrect ; it should be "==" to test if it's fully guessed. "<=" only tests that some of it has been guessed. \$\endgroup\$ – pjz Jan 16 at 17:14

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.