Hangman game in Python

Question

While I'm not new to Python, this is my first 'bigger' Python program I created, also my first game in Python.

import random

DEBUG = False; # If set to true, displays chosen word before each game. For debugging and cheating only :)

DICT_FILE = "english.txt";
GUESSES   = 10;


def prompt(s):
    while True:
        ans = raw_input(s + " (Y/n) ").lower();
        if ans in ["y", "n"]:
            return ans == "y";

def loadWords():
    with open(DICT_FILE, "r") as f:
        return f.read().strip().split("\n");
                       # Strip in case of newline at end of file

def play(word):
    word = word.lower();
    if DEBUG:
        print "[DEBUG] word is %r" % (word);
    guesses = GUESSES;
    toGuess = set(l for l in word.upper() if l.isalpha()); # Handles apostrophes, etc.
    letters = set();
    while guesses > 0:
        if len(toGuess) == 0:
            print "You win with %d guess%s left!" % (guesses, "" if guesses == 1 else "es");
            print "The word is %r" % (word);
            return True;
        print "=" * 78;
        print "You have %d guess%s left." % (guesses, "" if guesses == 1 else "es");
        print "Word:    " + " ".join("_" if c in toGuess else c for c in word.upper());
        print "Letters: " + " ".join(sorted(letters));
        while True:
            l = raw_input("Choose a letter: ").upper();
            if l.isalpha():
                if l not in letters:
                    letters.add(l);
                    if l in toGuess:
                        toGuess.remove(l);
                        print "Correct!";
                    else:
                        guesses -= 1;
                        print "Sorry, not in the word.";
                    break;
                else:
                    print "You already chose that letter!";
            else:
                print "That's not a letter!";
    print "=" * 78;
    print "Sorry, you lose.";
    print "The word was: %r" % (word);
    return False;

if __name__ == "__main__":
    words = loadWords();
    random.shuffle(words);
    while True:
        play(words.pop()); # Remove a random word so it's not chosen again
        if not prompt("Would you like to play again?"):
            break;

I'd like to know what you think and what I should change.
By the way, english.txt placed in the same folder as the game script contains phrases/words separated by newlines that should appear in the hangman game.

@Malachi Yes, as far as I know semicolons are optional in Python, but everything works fine with them. — Mateon1
– Mateon1, Commented Dec 22, 2014 at 15:57

Veedrac · Accepted Answer · 2014-12-25 17:15:23Z

I'm not the first person to wonder about those semicolons, but I might be the first to explain what they do!

They are used to teminate a line early, so that you can put two logical lines on the same physical one, such as in:

a = 1; b = 2

This is normally frowned upon (this in particular should be written a, b = 1, 2) and use where not needed is a very dangerous sight to the untrained Pythonista. Use with care.

From the the prompt function:

The parameter's name should be something meaningful like question.
Writiting (Y/n) with the Y in capitals has the standard meaning of "defaults to yes". If this is not wanted you should use (y/n).
Stylistically, you should either use a tuple or a set for if ans in ["y", "n"].

I would do:

def prompt(question):
    while True:
        ans = raw_input(question + " (Y/n) ") or "y"

        if ans.lower() in ("y", "n"):
            return ans == "y"

For loadWords:

The name breaks convention; it should probably be load_words.
It should not use the global directly, or it should use it as a default parameter.
You do too much work inside the with after you finish reading it. It's a good habit to minimize the are under contexts, whether they be trys or withs.
Don't read everything and then split it; utilize files being iterable.
What's up with the positioning of that comment?
You don't need the "r" parameter to open; it's the default.

I would do:

def loadWords(filename="english.txt"): with open(filename) as f: return [line.strip() for line in f if line]

and ditch the global.

For play

The function is too long; a standard function should be a little smaller than this. It's OK to have longer functions, but they need to be justified (for example, implementing a non-trivial algorithm).
The function does not have a single responsibility; it does both output and dealing with minor concerns like if l.isalpha().
"[DEBUG] word is %r" % (word) strangely has word in brackets. You should remove the brackets; they do nothing. Sometimes it makes sense to wrap it in a tuple ((word,)), but this is not what you did. IMHO it's even better to just print "[DEBUG] word is", repr(word).
You should probably avoid using an overarching while guesses > 0 loop; a while True with appropriate exits would be clearer since it would not imply that there is only one standard exit method.
toGuess should be to_guess, or IMHO just unguessed.
You can use a set comprehension for unguessed: {l for l in word.upper() if l.isalpha()}.
You're not checking for the length of the guess.
letters should be named something like guessed.
"{}".format is IMHO better than the old-style %s formatting.

I would thus choose something like:

import random

# If set to true, displays chosen word before each game.
# For debugging and cheating only :)
DEBUG = False
GUESSES = 10

def loadWords(filename="english.txt"):
    with open(filename) as file:
        return [line.strip() for line in file if line]

def prompt(question):
    while True:
        ans = raw_input(question + " (Y/n) ") or "y"

        if ans.lower() in {"y", "n"}:
            return ans == "y"

def get_guess(guessed):
    while True:
        guess = raw_input("Choose a letter: ").upper()

        if not guess.isalpha() or len(guess) != 1:
            print "That's not a letter!"
            continue

        if guess in guessed:
            print "You have already chosen that letter!"
            continue

        return guess

def alert_turn(word, guesses_left, guessed, unguessed):
    print "=" * 78
    print "You have {} guess{} left.".format(guesses_left, "" if guesses_left == 1 else "es")
    print "Word:   ", " ".join("_" if c in unguessed else c for c in word.upper())
    print "Letters:", " ".join(sorted(guessed))

def alert_correct():
    print "Correct!"

def alert_incorrect():
    print "Sorry, not in the word."

def alert_won(word, guesses_left):
    print "You win with {} guess{} left!".format(guesses_left, "" if guesses_left == 1 else "es")
    print "The word is", repr(word)

def alert_lost(word):
    print "=" * 78
    print "Sorry, you lose."
    print "The word was:", repr(word)

def play(word):
    word = word.lower()
    guessed = set()
    unguessed = {l for l in word.upper() if l.isalpha()}
    guesses_left = GUESSES

    if DEBUG:
        print "[DEBUG] word is", repr(word)

    while True:
        alert_turn(word, guesses_left, guessed, unguessed)
        guess = get_guess(guessed)
        guessed.add(guess)

        if guess in unguessed:
            alert_correct()
            unguessed.remove(guess)

            if not unguessed:
                alert_won(word, guesses_left)
                return True

        else:
            alert_incorrect()
            guesses_left -= 1

            if not guesses_left:
                alert_lost(word)                
                return False

if __name__ == "__main__":
    words = loadWords()
    random.shuffle(words)

    while True:
        # Remove a random word so it's not chosen again
        play(words.pop())

        if not prompt("Would you like to play again?"):
            break

Malachi · Accepted Answer · 2014-12-22 16:02:24Z

This is Python and you do not need the Semi-Colons (;)

Your code looks absolutely non-pythonic like that, it looks like it is trying to morph into Java or something.

You should also separate some more code into methods/functions like this one here

print "=" * 78;

I believe that this is to separate the game from the ending notifications.

What gave this away was 78 I have no clue what that is, and if I have different settings on my console/IDE then this won't look right anyway. 78 is a magic number and should be dealt with so that I know what it means

SylvainD · Accepted Answer · 2014-12-22 16:22:53Z

After a quick test, your code seems to be working fine.

Before going into interesting comments, let's go through quick details. Python has a style guide called PEP8. The document is definitly worth a read. Except if you have good reason not to, you should probably comply to this guideline. You'll find various tools to check you code like pep8 or to fix it autopep8.

Here, pep8 gives :

hangman.py:3:14: E703 statement ends with a semicolon
hangman.py:3:15: E261 at least two spaces before inline comment
hangman.py:3:80: E501 line too long (106 > 79 characters)
hangman.py:5:26: E703 statement ends with a semicolon
hangman.py:6:8: E221 multiple spaces before operator
hangman.py:6:15: E703 statement ends with a semicolon
hangman.py:11:47: E703 statement ends with a semicolon
hangman.py:13:30: E703 statement ends with a semicolon
hangman.py:15:1: E302 expected 2 blank lines, found 1
hangman.py:17:44: E703 statement ends with a semicolon
hangman.py:18:24: E111 indentation is not a multiple of four
hangman.py:18:24: E113 unexpected indentation
hangman.py:20:1: E302 expected 2 blank lines, found 1
hangman.py:21:24: E703 statement ends with a semicolon
hangman.py:23:44: E703 statement ends with a semicolon
hangman.py:24:22: E703 statement ends with a semicolon
hangman.py:25:58: E703 statement ends with a semicolon
hangman.py:25:59: E261 at least two spaces before inline comment
hangman.py:25:80: E501 line too long (86 > 79 characters)
hangman.py:26:20: E703 statement ends with a semicolon
hangman.py:29:80: E501 line too long (92 > 79 characters)
hangman.py:29:92: E703 statement ends with a semicolon
hangman.py:30:44: E703 statement ends with a semicolon
hangman.py:31:24: E703 statement ends with a semicolon
hangman.py:32:23: E703 statement ends with a semicolon
hangman.py:33:80: E501 line too long (84 > 79 characters)
hangman.py:33:84: E703 statement ends with a semicolon
hangman.py:34:80: E501 line too long (87 > 79 characters)
hangman.py:34:87: E703 statement ends with a semicolon
hangman.py:35:54: E703 statement ends with a semicolon
hangman.py:37:55: E703 statement ends with a semicolon
hangman.py:40:35: E703 statement ends with a semicolon
hangman.py:42:42: E703 statement ends with a semicolon
hangman.py:43:41: E703 statement ends with a semicolon
hangman.py:45:37: E703 statement ends with a semicolon
hangman.py:46:56: E703 statement ends with a semicolon
hangman.py:47:26: E703 statement ends with a semicolon
hangman.py:49:59: E703 statement ends with a semicolon
hangman.py:51:45: E703 statement ends with a semicolon
hangman.py:52:19: E703 statement ends with a semicolon
hangman.py:53:29: E703 statement ends with a semicolon
hangman.py:54:38: E703 statement ends with a semicolon
hangman.py:55:17: E703 statement ends with a semicolon
hangman.py:58:24: E703 statement ends with a semicolon
hangman.py:59:26: E703 statement ends with a semicolon
hangman.py:61:26: E703 statement ends with a semicolon
hangman.py:61:27: E261 at least two spaces before inline comment
hangman.py:63:18: E703 statement ends with a semicolon

Also, the names do not follow the naming convention.

After fixing this, the code looks like :

import random

DEBUG = False  # If set to true, displays chosen word before each game. For debugging and cheating only :)

DICT_FILE = "english.txt"
GUESSES = 10


def prompt(s):
    while True:
        ans = raw_input(s + " (Y/n) ").lower()
        if ans in ["y", "n"]:
            return ans == "y"


def load_words():
    with open(DICT_FILE, "r") as f:
        return f.read().strip().split("\n")  # Strip in case of newline at end of file


def play(word):
    word = word.lower()
    if DEBUG:
        print "[DEBUG] word is %r" % (word)
    guesses = GUESSES
    to_guess = set(l for l in word.upper() if l.isalpha())  # Handles apostrophes, etc.
    letters = set()
    while guesses > 0:
        if len(to_guess) == 0:
            print "You win with %d guess%s left!" % (guesses, "" if guesses == 1 else "es")
            print "The word is %r" % (word)
            return True
        print "=" * 78
        print "You have %d guess%s left." % (guesses, "" if guesses == 1 else "es")
        print "Word:    " + " ".join("_" if c in to_guess else c for c in word.upper())
        print "Letters: " + " ".join(sorted(letters))
        while True:
            l = raw_input("Choose a letter: ").upper()
            if l.isalpha():
                if l not in letters:
                    letters.add(l)
                    if l in to_guess:
                        to_guess.remove(l)
                        print "Correct!"
                    else:
                        guesses -= 1
                        print "Sorry, not in the word."
                    break
                else:
                    print "You already chose that letter!"
            else:
                print "That's not a letter!"
    print "=" * 78
    print "Sorry, you lose."
    print "The word was: %r" % (word)
    return False

if __name__ == "__main__":
    words = load_words()
    random.shuffle(words)
    while True:
        play(words.pop())  # Remove a random word so it's not chosen again
        if not prompt("Would you like to play again?"):
            break

Now, for the actual comments.

It would probably be worth writing a function to get the new letter. Something like :

def prompt_new_letter(s, letters):
    while True:
        l = raw_input(s).upper()
        if l.isalpha():
            if l not in letters:
                return l
            else:
                print "You already chose that letter!"
        else:
            print "That's not a letter!"

...

        l = prompt_new_letter("Choose a letter: ", letters)
        letters.add(l)
        if l in to_guess:
            to_guess.remove(l)
            print "Correct!"
        else:
            guesses -= 1
            print "Sorry, not in the word."

Not much else to say at the moment.

I was thinking about using a new function for asking for another letter, but I didn't think I should since I use the set of letters in the code. Anyway, I'll (leave the original code and) add the final, refactored code to my question once I think there is nothing else to say. — Mateon1
– Mateon1, Commented Dec 22, 2014 at 16:28
It is much a matter of preference to create or not a function here. I think having the new method separates the concern : you have the input logic outside the fame logic. As for the update of your question, "You also should not append your revised code to the question" as per meta.codereview.stackexchange.com/questions/1763/… . — SylvainD
– SylvainD, Commented Dec 22, 2014 at 16:33
Thank you for linking me that meta post. I'll combine both of the answers with everything I found helpful. — Mateon1
– Mateon1, Commented Dec 22, 2014 at 16:37

Mark Reed · Accepted Answer · 2014-12-22 17:01:57Z

First, I would get rid of the semicolons and the extra blank lines; they're distracting. A single blank line should be enough to break things up.

Other than that, the play function is a bit too long. Any time you have that much code in a single function or method, it's time to break it up a bit.

And as an aside, it seems a little odd to me that you first convert the word to lowercase, and then use uppercase for the actual letters. Seems like you could use just the one case conversion there.

Community · Accepted Answer · 2017-04-13 12:40:57Z

From what I've seen answered so far, my main issue is using semicolons after every statement and not following pep8. I've fixed this in my code by:

removing the semicolons, as Malachi and Josay pointed out,
formatting the comments to follow the pep8 guide,
Changing print "=" * 78 to print "===", a three character wide separator is enough, while removing magic numbers. The separator will now look good in any console/terminal, (As pointed out by Malachi), I don't think creating a new function is worth it, because this is a simple statement used only twice.
Renamed all word-related variables to phrase, the game can handle any phrase, not just a single word.
As Mark and Josay pointed out, my play() function is a bit too long. I broke it up a bit.
Mark pointed out, that I needlessly convert my phrase to lowercase, only to convert it to uppercase later. I changed it to be stored as uppercase.

Here's my final code:

import random

# If set to true, displays chosen word before each game.
# For debugging and cheating only :)
DEBUG = False

DICT_FILE = "english.txt"
GUESSES = 10


def prompt(s):
    while True:
        ans = raw_input(s + " (Y/n) ").lower()
        if ans in ["y", "n"]:
            return ans == "y"


def read_phrases():
    with open(DICT_FILE, "r") as f:
        return f.read().strip().split("\n")
        # Strip before splitting in case of newline at end of file


def prompt_letter(letters_so_far):
    while True:
        c = raw_input("Choose a letter: ").upper()
        if c.isalpha():
            if c not in letters_so_far:
                return c
            else:
                print "You already chose that letter!"
        else:
            print "That's not a letter!"


def play(phrase):
    phrase = phrase.upper()
    if DEBUG:
        print "[DEBUG] phrase is %r" % (word)
    guesses = GUESSES
    to_guess = set(l for l in phrase if l.isalpha())  # Handles apostrophes, etc.
    letters = set()
    while guesses > 0:
        if len(to_guess) == 0:
            print "You win with %d guess%s left!" % (guesses, "" if guesses == 1 else "es")
            print "The phrase is %r" % (phrase)
            return True
        print "==="
        print "You have %d guess%s left." % (guesses, "" if guesses == 1 else "es")
        print "Phrase:  " + " ".join("_" if c in to_guess else c for c in phrase)
        print "Letters: " + " ".join(sorted(letters))
        while True:
            c = prompt_letter(letters)
            letters.add(c)
            if c in to_guess:
                to_guess.remove(c)
                print "Correct!"
            else:
                guesses -= 1
                print "Sorry, not in the phrase."
            break
    print "==="
    print "Sorry, you lose."
    print "The phrase was: %r" % (phrase)
    return False

if __name__ == "__main__":
    phrases = read_phrases()
    random.shuffle(phrases)
    while True:
        play(phrases.pop())  # Remove a random phrase so it's not chosen again
        if not prompt("Would you like to play again?"):
            break

jsanc623 · Accepted Answer · 2014-12-22 17:51:59Z

Removed all semicolons - they are not required in Python
Removed all parentheses around the word variable as they're unnecessary
Renamed the toGuess variable in play() to to_guess
Renamed the loadWords function to load_words
Created new separator and msg functions to clean up print statements in code

To expand a bit, we are renaming the function and variables in order to align with PEP8 naming conventions.

Here's the code:

import random

DEBUG = False # If set to true, displays chosen word before each game.
DICT_FILE = "english.txt"
GUESSES   = 10

def prompt(s):
    while True:
        ans = raw_input(s + " (Y/n) ").lower()
        if ans in ["y", "n"]:
            return ans == "y"

def load_words():
    with open(DICT_FILE, "r") as f:
        return f.read().strip().split("\n") # Strip in case of newline at end of file

def separator():
    print "=" * 78

def msg(*message):
    for line in message:
        print line

def play(word):
    word = word.lower()
    if DEBUG:
        print "[DEBUG] word is %r" % word
    guesses = GUESSES
    to_guess = set(l for l in word.upper() if l.isalpha()) # Handles apostrophes, etc.
    letters = set()
    while guesses > 0:
        if len(to_guess) == 0:
            msg("You win with %d guess%s left!" % (guesses, "" if guesses == 1 else "es"), "The word is %r" % word)
            return True
        separator()
        msg("You have %d guess%s left." % (guesses, "" if guesses == 1 else "es"),
            "Word:    " + " ".join("_" if c in to_guess else c for c in word.upper()),
            "Letters: " + " ".join(sorted(letters)))
        while True:
            l = raw_input("Choose a letter: ").upper()
            if l.isalpha():
                if l not in letters:
                    letters.add(l)
                    if l in to_guess:
                        to_guess.remove(l)
                        msg("Correct!")
                    else:
                        guesses -= 1
                        msg("Sorry, not in the word.")
                    break
                else:
                    msg("You already chose that letter!")
            else:
                msg("That's not a letter!")
    separator()
    msg("Sorry, you lose.", "The word was: %r" % word)
    return False

if __name__ == "__main__":
    words = load_words()
    random.shuffle(words)
    while True:
        play(words.pop()) # Remove a random word so it's not chosen again
        if not prompt("Would you like to play again?"):
            break

I am not quite sure to see the point of if len(message) == 0: return False. If the length is 0, the function wouldn't do much anyway, would it ? — SylvainD
– SylvainD, Commented Dec 22, 2014 at 16:57
could you explain in your answer why you would rename those variables, it would make for a better review. — Malachi
– Malachi, Commented Dec 22, 2014 at 17:29
@Josay Good point - it's a personal preference of mine to return early if there's nothing to work with...a bit of a leftover from another language I work with :) removing — jsanc623
– jsanc623, Commented Dec 22, 2014 at 17:51

Stack Exchange Network

Hangman game in Python

6 Answers 6

You must log in to answer this question.

Hot Network Questions

Hangman game in Python

6 Answers 6

You must log in to answer this question.

Related

Hot Network Questions