Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
  • Buried in the middle of a long program, you hard-coded "test.txt". That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collectorrely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().
  • Buried in the middle of a long program, you hard-coded "test.txt". That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().
  • Buried in the middle of a long program, you hard-coded "test.txt". That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().
added 4 characters in body
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481

Here's how I would write it (without making too many of the recommendationsrecommended changes listed above).

Here's how I would write it (without making too many of the recommendations listed above).

Here's how I would write it (without making too many of the recommended changes listed above).

Changed gallows usage
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481
  • Buried in the middle of a long program, you hard-coded test"test.txttxt". That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().
def play(word):
    """ Play one game of Hangman for the given word.  Returns True if the
        player wins, False if the player loses. """
    imagegallows = Gallows()
    wrongGuesseswrong_guesses = 0                            # number of incorrect guesses
    currentImg = image.set_state(wrongGuesses)  # current state of the gallows
    blanks = set_blanks(word)                   # blanks which hide each letter of the word until guessed
    used = []                                   # list of used letters 

    while True:
        new_page()
        print(currentImggallows.get_image(wrong_guesses))
        print(' '.join(blanks))
        print(' '.join(used))

        guess = input("Guess a letter: ")
        blanks, used, missed = check_letter(word, guess.lower(), blanks, used)

        if blanks == list(word):
            return endgame(True, word)
        elif missed and wrongGuesseswrong_guesses >= 6:
            return endgame(False, word)
        elif missed:
            wrongGuesseswrong_guesses += 1
            currentImg = image.set_state(wrongGuesses)

def endgame(won, word):
    print('')
    if won:
        print("Congratulations, you win!")
        print("You correctly guessed the word '%s'!" % word)
    else:
        print("Nice try! Your word was '%s'." % word)
    return won

def play_again():
    while True:
        play_again = input("Play again? [y/n] ")
        if 'y' in play_again.lower():
            return True
        elif 'n' in play_again.lower():
            return False
        else:
            print("Huh?")


def main(words_file='test.txt'):
    wordlist = Wordlist(words_file)

    new_page()
    print("\nWelcome to Hangman!")
    print("Guess the word before the man is hanged and you win!")
    input("\n\t---Enter to Continue---\n")
    new_page()

    while True:
        play(wordlist.new_word())
        if not play_again():
            break
  • Buried in the middle of a long program, you hard-coded test.txt. That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().
def play(word):
    """ Play one game of Hangman for the given word.  Returns True if the
        player wins, False if the player loses. """
    image = Gallows()
    wrongGuesses = 0                            # number of incorrect guesses
    currentImg = image.set_state(wrongGuesses)  # current state of the gallows
    blanks = set_blanks(word)                   # blanks which hide each letter of the word until guessed
    used = []                                   # list of used letters
    while True:
        new_page()
        print(currentImg)
        print(' '.join(blanks))
        print(' '.join(used))

        guess = input("Guess a letter: ")
        blanks, used, missed = check_letter(word, guess.lower(), blanks, used)

        if blanks == list(word):
            return endgame(True, word)
        elif missed and wrongGuesses >= 6:
            return endgame(False, word)
        elif missed:
            wrongGuesses += 1
            currentImg = image.set_state(wrongGuesses)

def endgame(won, word):
    print('')
    if won:
        print("Congratulations, you win!")
        print("You correctly guessed the word '%s'!" % word)
    else:
        print("Nice try! Your word was '%s'." % word)
    return won

def play_again():
    while True:
        play_again = input("Play again? [y/n] ")
        if 'y' in play_again.lower():
            return True
        elif 'n' in play_again.lower():
            return False
        else:
            print("Huh?")


def main(words_file='test.txt'):
    wordlist = Wordlist(words_file)

    new_page()
    print("\nWelcome to Hangman!")
    print("Guess the word before the man is hanged and you win!")
    input("\n\t---Enter to Continue---\n")
    new_page()

    while True:
        play(wordlist.new_word())
        if not play_again():
            break
  • Buried in the middle of a long program, you hard-coded "test.txt". That's bad for maintainability; hard-coding the same filename twice is even worse. Instead, the WordList constructor should take a filename parameter.
  • You might have a file descriptor leak in the constructor, since you open() the file not using a with block or calling close(). Therefore, you rely on the garbage collector to trigger the closing, which won't necessarily happen in all Python interpreters.
  • No need to assign the variable word. Just return line.lower().strip().
def play(word):
    """ Play one game of Hangman for the given word.  Returns True if the
        player wins, False if the player loses. """
    gallows = Gallows()
    wrong_guesses = 0                           # number of incorrect guesses
    blanks = set_blanks(word)                   # blanks which hide each letter of the word until guessed
    used = []                                   # list of used letters 

    while True:
        new_page()
        print(gallows.get_image(wrong_guesses))
        print(' '.join(blanks))
        print(' '.join(used))

        guess = input("Guess a letter: ")
        blanks, used, missed = check_letter(word, guess.lower(), blanks, used)

        if blanks == list(word):
            return endgame(True, word)
        elif missed and wrong_guesses >= 6:
            return endgame(False, word)
        elif missed:
            wrong_guesses += 1

def endgame(won, word):
    print('')
    if won:
        print("Congratulations, you win!")
        print("You correctly guessed the word '%s'!" % word)
    else:
        print("Nice try! Your word was '%s'." % word)
    return won

def play_again():
    while True:
        play_again = input("Play again? [y/n] ")
        if 'y' in play_again.lower():
            return True
        elif 'n' in play_again.lower():
            return False
        else:
            print("Huh?")


def main(words_file='test.txt'):
    wordlist = Wordlist(words_file)

    new_page()
    print("\nWelcome to Hangman!")
    print("Guess the word before the man is hanged and you win!")
    input("\n\t---Enter to Continue---\n")
    new_page()

    while True:
        play(wordlist.new_word())
        if not play_again():
            break
Mention file descriptor leak
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481
Loading
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481
Loading