I recently started learning Python, and made a simple text based Hangman game. I wanted to know if there could be anything done to make the code cleaner or more efficient.
import random
import os
import sys
def main():
pickword()
def start(word, twoplayer):
wordarray = ''
userguess = []
guessright = 0
lettersright = 0
if len(word) < 5:
tries = 6
else:
tries = 10
usedletters = []
if twoplayer == False:
print(word)
print("Welcome to hangman")
print("I will think of a word and you will have to guess it, are you ready!")
input("Press enter to continue")
else:
print("Welcome to hangman")
print("Player 1 has already thought of a word, let's begin")
input("Press enter to continue")
lengthword = len(word)
if twoplayer == False:
for i in range(1, lengthword): #creates a new variable, all underscores
userguess.append("_ ")
else:
for i in range(1, lengthword): #creates a new variable, all underscores
userguess.append("_ ")
wordarray = (list(word))
if twoplayer == False:
for i in range(0, lengthword): #removes \n from end of word
if wordarray[i] == "\n":
wordarray.remove(wordarray[i])
break
usedletters = []
getuserguess(wordarray, userguess, tries, usedletters, word)
def pickword():
word = 0
twoplayer = input("Do you want to play 2 player[Y/N]")
if twoplayer.lower() == "y":
word = input("Player 1, enter the word you want player 2 to guess, don't let them see! ")
if len(word) < 3:
print("That word is too short")
pickword()
print("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n")
print("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n")
print("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n")
word = word.lower()
twoplayer = True
start(word, twoplayer)
else:
try:
word = random.choice(open('hangman.txt').readlines())
twoplayer = False
start(word, twoplayer)
except:
input("Error, no word list found, press enter to exit")
sys.exit()
def getuserguess(wordarray, userguess, tries, usedletters, word):
print(' '.join(userguess))
guess1 = input("Enter your guess: ")
if len(guess1) < 1 or len(guess1) > 1 or guess1.isdigit():
print("Invalid input")
getuserguess(wordarray, userguess, tries, usedletters, word)
else:
checkuserguess(wordarray, userguess, guess1, usedletters, tries, word)
def lose(word):
again = input("Sorry, you lost, the word was {}Would you like to play again[Y/N]".format(word))
if again.lower() == "y":
pickword()
else:
sys.exit()
def win():
again = input("Congrats, you won hangman, do you want to play again?[Y/N]")
if again == "Y" or "y":
pickword()
else:
sys.exit()
def checkuserguess(wordarray, userguess, guess1, usedletters, tries, word):
guess1list = (list(guess1))
if guess1list[0] in usedletters:
print("You already used that letter")
getuserguess(wordarray, userguess, tries, usedletters, word)
if guess1 in wordarray:
usedletters.append(guess1)
for i in range(0, len(wordarray)):
if wordarray[i] == guess1list[0]:
userguess[i] = guess1list[0]
usedletters.append(guess1list[0])
if "_ " not in userguess:
print(' '.join(userguess))
win()
else:
getuserguess(wordarray, userguess, tries, usedletters, word)
else:
if tries > 0:
print("Sorry, that letter is not in the word, you have ", tries, " remaining")
tries -= 1
getuserguess(wordarray, userguess, tries, usedletters, word)
else:
lose(word)
main()
1 Answer 1
For this, I am going to go through your program in the order it is written:
Don't use
if twoplayer == False:
. You can simply sayif not twoplayer:
You don't even need that, though the first time because you do the same thing in theif
andelse
blocks`.You can use
" ".join("_" * lengthword)
instead offor i in range(1, lengthword): userguess.append("_ ")
. Besides making it cleaner, you weren't adding enough underscores. The second argument torange()
is exclusive. Imagine if the length of the word is two. In that case,range(1, 2)
returns[1]
because the second argument is exclusive. That has length 1 instead of the same length as the word: 2.Don't use
wordarray.remove(wordarray[i])
. Instead, usewordarray.pop(i)
. If you use.remove()
, the first occurrence of the word will be removed, not necessarily the one you want to be removed.When you ask
"Do you want to play 2 player?"
, you account for"y"
and anything else. What if the user says"yes"
? It would still count as no. Instead, you should use awhile
loop:twoplayer = input("Do you want to play 2 player? [Y/N] ") while twoplayer.lower() not in ("y", "n"): twoplayer = input("[Y/N] ")
Instead of setting
twoplayer
inside of theif
andelse
blocks, you can just dotwoplayer = ("n", "y").index(twoplayer.lower())
If the word is too short, you don't want to call
pickword()
again becausepickword()
asks"Do you want to play 2 player?"
If the word is too short, that doesn't mean the user wants to change his mind on that question. Instead, use a loop:while len(word) < 3: print("That word is too short") word = input("Player 1, enter the word you want player 2 to guess, don't let them see! ")
How do you know how many lines to print? Instead of printing an arbitrary number of lines,
print("033円c")
will clear the terminal.You call
start(word, twoplayer)
in both theif
and theelse
block. That is a sign that it shouldn't be in either one. Put it after both blocks so that you aren't duplicating code.While we're at that point in the program, you want only one line in your
try:
block:word = random.choice(...)
.twoplayer
should be defined as I outlined above, so we don't need that here at all andstart(word, twoplayer)
will be outside of theif
andelse
blocks, so we don't need that. Besides, the only reason you want thetry
block is because you are dealing with a file. If anything else is throwing an error, you want the traceback because it shouldn't be.Don't use a bare
except:
You have certain errors that you think you might get; list them here. In this case, it'sexcept IOError:
sys.exit()
, when given an argument, uses that argument as the exit code. In this case it is failure, so you should usesys.exit(1)
.Your check for
guess1
's validity is not the best. To make sure that it is one character, uselen(guess1) != 1
as the check. Also,guess1.isdigit()
is probably not the only thing that is invalid. For example,$
shouldn't be in the word, right? Instead, usenot guess1.isalpha()
to make sure it is an alphabetic character.Instead of using recursion and typing out all of those variable names, use a loop:
print(' '.join(userguess)) guess = input("Enter your guess: ") while len(guess) != 1 or not guess.isalpha(): print("Invalid input") guess = input("Try again: ") checkuserguess(wordarray, userguess, guess, usedletters, tries, word)
You have duplicate code in
lose()
andwin()
:Would you like to play again
. Instead, define a function that asks that question, and use it inlose()
andwin()
. Make sure that in that function you useif again.lower() == "y":
as you did inlose()
.if again == "Y" or "y":
does not work how you would expect it to. See this StackOverflow post for why.You check if
"_ "
is inuserguess
, but you should be checking for just the underscore, not the underscore with a space. If the underscore is at the end of the word, there shouldn't be a space. If there is, you're doing it wrong and you shouldn't depend on it.
This is my first complete review, so if anyone has suggestions, I would be happy to modify it.
-
\$\begingroup\$ Thanks for the input, I understand everything except the part where you say use twoplayer = ("n", "y").index(twoplayer.lower()). Could you explain this a little bit more. \$\endgroup\$vkumar– vkumar2016年03月13日 21:48:43 +00:00Commented Mar 13, 2016 at 21:48
-
\$\begingroup\$
twoplayer.lower()
will be either"y"
or"n"
. If it is"y"
, then("n", "y").index(twoplayer.lower())
will be 1. If it is"n"
, then it will be 0. Since 1 has a boolean value ofTrue
and 0 has a boolean value ofFalse
,twoplayer
can now be used as a boolean. \$\endgroup\$zondo– zondo2016年03月13日 21:50:45 +00:00Commented Mar 13, 2016 at 21:50 -
\$\begingroup\$ One more thing, could you please explain how I should use " ".join("_" * lengthword) \$\endgroup\$vkumar– vkumar2016年03月13日 22:02:39 +00:00Commented Mar 13, 2016 at 22:02
-
\$\begingroup\$ You can take out the
for
loop and put that in. I edited my answer to make that more clear and to add one more thing on that. \$\endgroup\$zondo– zondo2016年03月13日 22:06:09 +00:00Commented Mar 13, 2016 at 22:06 -
\$\begingroup\$ Might just make it that little clearer for twoplayer = ("n", "y").index(twoplayer.lower()) to use twoplayer = bool(("n", "y").index(twoplayer.lower())) to make it explicit that the index will be a boolean value? Great work @zondo ! \$\endgroup\$srattigan– srattigan2017年08月31日 18:39:17 +00:00Commented Aug 31, 2017 at 18:39