# Hangman game Python project

This is one of my first programmes which I had written (definitely first over 100 lines) and I could really use some feedback.

import time
import getpass
import random

def invalid():
print("")
print("INVALID INPUT")
time.sleep(sec)

def play_game(secret,sec): #the actual game
def correct_guess(guess,secretword,output):
j=[]
t=-1 #Next couple lines is modefied code from https://stackoverflow.com/questions/20039022/python-finding-more-than-one-index-in-a-list-of-letters
while True:
try:
t=secretword.index(guess,t+1)
j.append(t)
except ValueError:
break
for z in j:
output[z]=guess

def draw_hangman(attempts): #generates image of hangman [attempts = number of wrong answers (up to 9)]
def legs(at):
if at==6:
if p==10:
print("*              *")
print("* *           *")
print("*  *")
if at>=7:
if p==10:
print("*              * *")
print("* *           *   *")
print("*   *")

def rest(att):
if att in (1,2,3,4,5):
n={1: 2,2: 3,3: 5,4: 7,5: 10}
c=n[att]
if p in range(c,11):
print("*")
if p==11:
print("* *")
print("*   *")

for  p in range(1,13):
if p==1:
print("")
if attempts>=2:
if p==2:
print("* * * * * * * * *")
if attempts>=3:
if p in (3,4):
print("*               *")
if attempts>=4:
if p==5:
print("*             *   *")
print("*               *")
if attempts in (5,6,7):
if p in (7,8,9):
print("*               *")
legs(attempts)
if attempts==8:
if p==7:
print("*             * *")
print("*            *  *")
print("*               *")
legs(attempts)
if attempts==9:
if p==7:
print("*             * * *")
print("*            *  *  *")
print("*               *")
legs(attempts)
else:
rest(attempts)
else:
rest(attempts)
else:
rest(attempts)
else:
rest(attempts)
#secret = is the secret word / word = what the player already guessed
word=[]
for _ in secret:
word.append("_")
k=0
check=[] #here go all the guesses to make sure something isn't guessed twice
while True:
if secret==word:
print("")
print(word)
print("")
print("VICTORY")
break
continue
else:
print("")
print(word)
print("")
g=input("Enter Guess:")

if len(g)!=1:
invalid()
continue
if " " in g:
invalid()
continue
if g in check:
print("")
time.sleep(sec)
continue
else:
check.append(g)
u=False
if g.lower() in secret:
correct_guess(g.lower(),secret,word)
u=True
if g.upper() in secret:
correct_guess(g.upper(),secret,word)
u=True
if u:
continue
else:
k=k+1
draw_hangman(k)
if k==9:
print("")
print(secret)
print("")
print("GAME OVER")
break
time.sleep(sec)

# got this list from https://github.com/Xethron/Hangman/blob/master/words.txt
sec=0.8 #since all time delays are the same I wanted them all to be a variable so it is easeier to change it
while True:
print("")
print("1:New Game(Random Word)")
print("3:Exit Game")
x=input("Enter(1,2,3):")
if x=="2":
while True:
print("")
wordchoice=getpass.getpass("Enter secret word:")
y=False
if " " in wordchoice:
print("")
print("Only one word!")
time.sleep(sec)
continue
if len(wordchoice)<1:
invalid()
continue
alphabet="abcdefghijklmnopqrstuvwxyz"
wordlist=list(wordchoice)
for n in wordlist:
if n in alphabet.lower():
continue
if n in alphabet.upper():
continue
else:
print("")
print("Use only letters!")
time.sleep(sec)
y=True
break
if y:
continue
else:
play_game(wordlist,sec)
break
continue
if x=="1":
wordlist=list(random.choice(wordlibary))
play_game(wordlist,sec)
continue
if x=="3":
break
else:
invalid()

• Generally, we prefer you to keep your original code in the question; this allows others to see the process. If you want the new code to be reviewed, you can ask a new question. In any case, please revert your most recent edit. – Graham Nov 17 '18 at 23:30
• Adding a revised version of your code is also discouraged, per this meta Code Review answer ("You also should not append your revised code to the question"), however I think I can show you a few more things on my answer before there's a need to ask a new question. – Graham Nov 17 '18 at 23:49
• Even if you want to learn by doing (which is a good philosophy), I would recommend going back and comparing your code to my code line by line afterwards. Figure out why I did something a certain way. If your code is longer, then it's almost certainly less clear (although that's not necessarily bad in the general case if it's a necessary optimization, but that will not be the case for this code.) If you can't figure out why I did something, leave a comment. If you want to hone the code down even further (which is certainly possible; I improved it, but there's other things), ask a new question. – Graham Nov 18 '18 at 0:10
• I would just recommend making sure you understand my code entirely, and have good reason for any deviations before asking a new question. – Graham Nov 18 '18 at 0:11
• Welcome to Code Review. I have rolled back your edits. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. – Heslacher Nov 19 '18 at 6:09

# Read this entire section first

Welcome to Code Review! I enjoyed trying out your game and playing it. Don't be discouraged by my long list of improvements; there's always a lot to learn when you're just starting, and every new thing you try brings its own challenges and best practices.

I would recommend reading the entire list and trying to understand the purpose of each of the suggestions. I have provided a revised version of your code at the end of my answer. The list is not ordered by importance, just by my flow when I was reviewing your code. Hopefully the logical order makes sense, and you'll be able to use this answer as a reference.

# Observe PEP-8 standards

PEP-8 is the widely-accepted style guide for Python and it makes Python code easier to read and more standard. It's basically the common language of Python programmers. I would recommend beginning to read it and learning the different guidelines. Here are a few style suggestions (The small text in parenthesis is regex replacements. You probably don't need to worry about them; they're just my quick way of fixing code in a text editor. For those who are familiar with regex, I always preview all the changes before applying the regex.):

• I notice you often do not put spaces on both sides of your = and comparison operators (==, !=, >=, and <=). PEP-8 recommends putting spaces on both sides would make your code easier to read. (I fixed this using the regex ?([><!]?=+) ? and replacing with \1.)
• I would also recommend using more descriptive variable names in certain places throughout your code: on first glance, I'm not sure what j, t, and z represent.
• Comments should generally be on a separate line from code, and avoided by using more clear code where possible (I'm referring to the comment after the draw_hangman declaration.)
• Almost universally, you should have spaces after commas. Sometimes it's readable in function argument lists, but for declaring a dict or list, it makes the code much more readable. (This is easy to fix using regex: ,(?! ) to ,)

# Nested methods

I notice you have a method hierarchy:

def play_game():
# ...
def correct_guess()
# ...
def draw_hangman()
def legs():
# ...
def rest():
# ...


Unless you want to restrict the scope of methods for a good reason, you shouldn't use nested methods. Just lay them flat on the same level in your file. If this makes the purpose of a method unclear, rename it and utilize docstrings. However, I think you partly did this to take advantage of being able to reference the function-scoped variable within another function. Instead, just pass the required variables as function arguments to the previous subfunction, and add those parameter the previous subfunction. For example, you reference p in legs() and rest(): just add p as a parameter to legs() and rest().

Let's talk about draw_hangman():

def draw_hangman(attempts):
def legs(at):
# ...

def rest(att):
# ...

for p in range(1, 13):
if p == 1:
print("")
if attempts >= 2:
if p == 2:
print("* * * * * * * * *")
if attempts >= 3:
if p in (3, 4):
print("*               *")
if attempts >= 4:
# ...
else:
rest(attempts)
else:
rest(attempts)
else:
rest(attempts)
else:
rest(attempts)


Firstly, your for p in range(1, 13) and generally the existence of the p variable is a bad pattern. All you're doing is printing out a set of lines in order; checking for where p is just makes your code unnecessarily inefficient and confusing to follow. As a general rule, you should never iterate through a range and then check your current position in the range to create conditionals; you should just do the things you need to do in order, and make conditional checks within at the appropriate points to change the output for specific parts. Refactoring this is complicated, but fortunately it will be unnecessary, as we'll get to in my next paragraph.

Instead of using conditionals to determine how to print the picture, you could simply encode the picture in an iterative way and then retrieve it conditionally. Let me show you what I mean; you can start with original hangman graphic:

* * * * * * * * *
*               *
*               *
*             *   *
*               *
*             * * *
*            *  *  *
*               *
*              * *
* *           *   *
*   *


and then number it corresponding to when specific parts should be displayed:

1 2 2 2 2 2 2 2 2
1               3
1               3
1             4   4
1               4
1             8 5 9
1            8  5  9
1               5
1              6 7
1 1           6   7
1   1


Conveniently, your hangman happens to have few enough parts to allow for string storage to with digits 1-9 to work; otherwise you would have to store the hangman image in a list.

You can then retrieve the image using string replacement. The python regex module re is particularly convenient, as I will demonstrate in the following example:

import re

# ...

hangman_image = """
1 2 2 2 2 2 2 2 2
1               3
1               3
1             4   4
1               4
1             8 5 9
1            8  5  9
1               5
1              6 7
1 1           6   7
1   1"""

# generates image of hangman [attempts = number of wrong answers (up to 9)]
def draw_hangman(attempts):
current_hangman = hangman_image
current_hangman = re.sub('[1-{}]'.format(attempts), '*', current_hangman)
current_hangman = re.sub('[{}-9]'.format(attempts + 1), ' ', current_hangman)
print(current_hangman)


The first time re.sub() is called, the regular expression replaces all numbers in the range 1 to attempts. The value of attempts is inserted into the string [1-{}] by using .format() to replace {} with the value of attempts. In regular expressions, brackets surrounding a concatenation of characters form what is called a "character class": this just means it's looking for a single character inside the brackets and then replacing it. Character classes support ranges of characters (relying on the way characters are encoded) by using characters separated by a hyphen for both ends of the range. The second time re.sub() is called is similar, but it instead targets all the numbers which are greater than the number of attempts and replaces them with spaces.

This conveniently also eliminates the need for the legs() and rest() functions.

# Main loop

To start with, kudos for making sec a variable. Generally, when you have repeated constants you should make them variables. Even if they aren't repeated, making them a variable can make procedured more clear (see Magic constant / Magic number).

Your main loop and your code generally needs more clear control flow. Instead of having multiple if clauses for mutually exclusive outcomes, you can use elif and else to direct mutually exclusive outcomes.

if x == "2":
# ...
elif x == "1":
# ...
elif x == "3":
break


Then, you need to clean up all your continues and breaks; an easy immediate change is to remove the continues and breaks preceding the each x comparison conditional. As you're using them now, they are a form of goto, and as Edgar Dijkstra famously noted in a paper, Go To Statement Considered Harmful (this is not to say all continues and breaks are bad, but you should probably minimize their use when possible.) Generally, you should use else more to obviate the need to use continues and breaks.

I will describe the refactoring process below:

The main revisions are inside if x == 2:

• Made the if statements checking the wordchoice into a chain of if elif statements, and then moved the remaining code in the while True loop into an else clause. The continues within the first two conditionals can now be removed.
• Python provides the lowercase and uppercase Roman alphabet (A.K.A. the English alphabet) as an easy import: just add from string import ascii_letters; however this is not even necessary...
• EDIT: As you correctly pointed out, my original regex method (\w+) didn't exclude numbers. To check if a word contains just ascii characters, just use the follow regular expression: [a-zA-Z]+. You can compare a string using re.fullmatch. Refactoring the code to use this will eliminate the need for the y variable.
• In the invalid() function, it's unclear where sec comes from. sec should passed as a parameter to invalid(). It's poor form to reference a global variable where the origin of the variable is unclear.
• As a matter of simplicity, just using a list of lowercase words will simplify things, since you don't check for case inside the play_game(); moving the list conversion of the word inside play_game() would also be a good idea to separate abstraction.

# Improving playgame()

Here, the problems are similar to the problems with your main loop; your control flow is very unclear. Your variable names could also use some work.

• A list comprehension can allow you to declare word in one line: word = ["_" for _ in secret]
• A continue directly preceded by a break does nothing. The continue will never be reached because the break will change the control flow.
• You should rename:
• k to wrong_guesses
• check to guessed_letters
• g to guess.
• Multiple consecutive conditionals should never all have continues within them. Just use else and elif; you can use or to join two conditions that have the same result.
• Instead of doing wrong_guesses = wrong_guesses + 1, you can increment using += like wrong_guesses += 1.

I would get rid of correct_guess, because you're trying to rely on list mutation to modify a variable. Instead of passing a list and mutating it, you should just do the changes inline as follows (this is also coupled with a change to make secret all lowercase in the beginning of playgame):

        if guess.lower() in secret or guess.upper() in secret:
for i in range(len(secret)):
if guess == secret[i]:
word[i] = guess


# Final code

I've tested the following code and it works! It can hopefully illustrate how to do a better form of control flow and modularization.

import time
import getpass
import random
import re

def invalid(sec):
print("")
print("INVALID INPUT")
time.sleep(sec)

hangman_image = """
1 2 2 2 2 2 2 2 2
1               3
1               3
1             4   4
1               4
1             8 5 9
1            8  5  9
1               5
1              6 7
1 1           6   7
1   1"""

# generates image of hangman [attempts = number of wrong answers (up to 9)]
def draw_hangman(attempts):
current_hangman = hangman_image
current_hangman = re.sub('[1-{}]'.format(attempts), '*', current_hangman)
current_hangman = re.sub('[{}-9]'.format(attempts + 1), ' ', current_hangman)
print(current_hangman)

def play_game(target_word, sec): #the actual game
secret = list(target_word.lower())
word = ["_" for _ in secret]
wrong_guesses = 0
guessed_letters = []
while True:
print()
print(word)
print()
guess = None
while True:
guess = input("Enter Guess:")
if len(guess) != 1 or " " in guess:
invalid(sec)
elif guess in guessed_letters:
print()
time.sleep(sec)
else:
break
guessed_letters.append(guess)
if guess.lower() in secret or guess.upper() in secret:
for i in range(len(secret)):
if guess == secret[i]:
word[i] = guess
else:
wrong_guesses = wrong_guesses + 1
draw_hangman(wrong_guesses)
if wrong_guesses == 9:
print()
print(secret)
print()
print("GAME OVER")
break
time.sleep(sec)
if word == secret:
print()
print(target_word)
print()
print("VICTORY")
return

# got this list from https://github.com/Xethron/Hangman/blob/master/words.txt

# since all time delays are the same I wanted them all to be a variable so it is easier to change it
sec = 0.8
while True:
print()
print("1:New Game(Random Word)")
print("3:Exit Game")
x = input("Enter(1, 2, 3):")
if x == "2":
wordchoice = None
while True:
print()
wordchoice = getpass.getpass("Enter secret word:")
print()
if " " in wordchoice:
print("Only one word!")
time.sleep(sec)
elif len(wordchoice) < 1:
print("Enter more than zero characters.")
elif not re.fullmatch('[a-zA-Z]+', wordchoice):
print("Use only letters!")
else:
break
time.sleep(sec)
play_game(wordchoice, sec)
elif x == "1":
play_game(random.choice(wordlibrary), sec)
elif x == "3":
break
else:
invalid(sec)


# Miscellaneous notes

• I would move wordlibrary to a separate file and then import it. Or read it from a file. Also, you misspelled "wordlibrary" as "wordlibary".
• I suggested this because it takes up a lot of space in your current file, and it's not something you need to look at when you're editing the code of the file; it's really an entirely separate component.
• If you're curious, I could also tell you a way to make your print() statements more succinct, but I didn't want to get into it in the main review.
• The easiest way to make print() statements more succint is to take advantage of newline characters and formatted string literals. For example if you wanted to turn the three lines print(), print(secret), print() into one line, you would do: print(f'\n{secret}\n')
• Thank you for your feedback! I implemented most of your suggestions into my code, however, didn't copy your final code since I wanted to practice using your final code only as a reference when necessary. Something worth mentioning is that (elif not re.fullmatch('\w+', wordchoice):) doesn't seem to work when I implement it in my code or if I run yours because when I tried entering numbers it didn't print use only letters. Couldn't figure out why. Also what would be the advantages of wordlibrary being a seperate file? And yes I am curious how I could make my print() succinct. Thanks for feedback – Jonas Nov 17 '18 at 23:44
• You're correct, I accidentally made a mistake with the regex. You can fix the regex problem by replacing \w+ with [a-zA-z]+; I've edited that into my answer, along with explanations for the other two parts. – Graham Nov 18 '18 at 0:01