HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
butmessyhangmanonegamecorrectfunctions

Problem

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.

Solution

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.

Code Snippets

def isWordGuessed(secretWord, lettersGuessed):
    if secretWord:
        if secretWord[0] in lettersGuessed:
            return isWordGuessed(secretWord[1:], lettersGuessed)
        else:
            return False
    return True
def isWordGuessed(secretWord, lettersGuessed):
    if secretWord:
        return secretWord[0] in lettersGuessed and isWordGuessed(secretWord[1:], lettersGuessed)
    return True
def isWordGuessed(secretWord, lettersGuessed):
    return not secretWord or secretWord[0] in lettersGuessed and isWordGuessed(secretWord[1:], lettersGuessed)
def isWordGuessed(secretWord, lettersGuessed):
    for letter in secretWord:
        if letter not in lettersGuessed:
            return False
    return True

Context

StackExchange Code Review Q#23375, answer score: 7

Revisions (0)

No revisions yet.