In my Hangman game, one of my functions is correct but messy

Question

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.

Answer 1

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.

Answer 2

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.