I'm quite new to Python and have been experimenting. So far I have made a simple hangman type game which creates a random word and the user then guesses it. I'd like to know if I've committed any bad practices so I can learn early on what to and not-to do.

#Word Game Python
from random import randint
#Define an alphabet of valid letters
alphabet_file = "alphabet.txt"
#Assign that to a list
alphabet = open(alphabet_file).read().splitlines()
#Define a word
#Find the dictionary
word_file = "words.txt"
#Assign the dictionary to a list
words = open(word_file).read().splitlines()
#Define word based off a random word in the dictionary
word = words[randint(0,len(words))]
#Make the word lower case
word = word.lower()
#Make a list of guesses
guesses = []
#Make a list of correct guesses
correctGuesses = []
#Make a list of incorrect guesses
incorrectGuesses = []
#Set a maximum number of incorrect guesses
incorrectGuessesMax = 7
#A function to help check if something is a number
def isNumber (n):
try:
float(n)
return True
except ValueError:
return False
#Returns True if a letter is in a word
def isInWord (letter,word):
#Make sure letter and word are both strings
if not letter in alphabet:
return False
#Make the letter lower case
letter = letter.lower()
#check if the letter is in the word
if letter in word:
#if so return true
return True
else:
#Otherwise return false
return False
#Returns how many times a letter appears in a word
def timesInWord (letter,word):
#Make the letter Lower Case
letter = letter.lower()
#Define a variable to store the times letter is found
letterCount = 0
#Check if the letter is in the word
if isInWord(letter,word):
#Loop through word to find letter
for l in range(0,len(word)):
#If the letter is found
if letter == word[l]:
#Add 1 to letterCount
letterCount += 1
#Add the guess to the list of correct guesses
if not isInWord(letter,guesses):
correctGuesses.append(letter)
guesses.append(letter)
#Return the time the letter was found
return letterCount
else:
if not isInWord(letter,guesses):
guesses.append(letter)
incorrectGuesses.append(letter)
return 0
#Ask the user to input a letter
def getLetter ():
letter = input("Please enter a letter ")
#Make sure it's actually a letter(is not a number)
x = letter
if isNumber(x):
print ("Invalid")
return getLetter()
#Make sure it's only one letter
#If it's a word it's ok, that will be chacked later
if len(letter) == 0:
print("Invalid")
return getLetter()
#If everything is ok, return the letter
return letter
#A function to print the word based on correct guesses
def printWord(word,guesses):
#define a printout variable
printout = ""
#loop through each letter in the word to check it against the guesses
for i in range(0,len(word)):
if word[i] in guesses:
#if there's a match add the letter to the printout
printout += word[i]
else:
#otherwise fill it with an _
printout += "_"
return printout
#Check if word is complete
def checkWord(word,guesses):
if word == printWord(word,guesses):
return True
else:
return False
#While the word is not complete keep asking for an input
while checkWord(word,correctGuesses) == False:
#Terminate while loop if there are to many incorrect guesses
if len( incorrectGuesses ) > incorrectGuessesMax:
print ("Too many incorrect guesses, the word was: " + word)
break
#Receive an input
guess = getLetter()
#Chack if the input was a letter or word
if len(guess) > 1:
#Check if the guess is equal to the word
if guess == word:
#Inform the user of their success
print("Well Done, You guessed the word correctly in " + str(len(guesses)) + " guesses!")
#Terminate the loop
break
else:
print(guess + " is not the word you are looking for")
else:
#Print how many letters are correct
print(str(timesInWord(guess,word)) + " out of " + str(len(word)))
#Print letter guesses
print ("Your guesses are: " + str(guesses))
#Print the amount of guesses left
guessesLeft = incorrectGuessesMax - len(incorrectGuesses)
guessesLeft += 1
print ("You have " + str(guessesLeft) + " incorrect guesses left")
#Print the current state of the word based on correct guesses
print(printWord(word,correctGuesses))
#Check if the word is completed
if printWord(word,correctGuesses) == word:
#Inform the user of their success
print("Well Done, You guessed the word correctly in " + str(len(guesses)) + " guesses!")
#Terminate the loop
break

2 Answers
2

The first thing that strikes me with your code are the comments: there are way too many comments. There are especially too many comments considering that your code rather well self-documented. For example, you could remove all the comments in the following snippets since they don't add any value to the code and provide no more information than the variables' names:

The PEP8 explicitely says that comments that contradicts the code (that's often the case when you modified the code but not the comments) is worse than no comments

The function isInWord can actually be simplified to:

letter.lower() in word

All the other checks are not really useful. The first check to check whether the letter is in the alphabet is not really useful, because the letter cannot be in the word in the first place if it is not in the alphabet (unless you did something wrong, but if you did something wrong, it will be simpler to read a meaningful error message).

This piece of code is rather misleading:

for l in range(0,len(word)):
#If the letter is found
if letter == word[l]:

It is not a good idea to name a variable l. When I read word[l], I was convinced that it was written word[1]. Try to avoid as much as possible variable names that can be read as numbers such as l, o, l2, lo, etc...

\$\begingroup\$Thank You!, on the point about isalpha() Many of the words contain apostrophe's, "'".isalpha --> False I solved this by using the alphabet.txt witch is, obviously, a text file containing every letter of the English Aplphabet along with apostrophe's. I know I could put in the apostrophes in for the user but I don't want to. Other than that thank you so much for the help and tips.\$\endgroup\$
– Everless Drop 41Aug 3 '14 at 16:13

Python fluency

#check if the letter is in the word
if letter in word:
#if so return true
return True
else:
#Otherwise return false
return False

You could just say return letter in word.

The implementation of timesInWord() could be simplified using a collections.Counter:

>>> Counter('abracadabra')['a']
5

Correctness and maintainability

Why bother implementing isInWord(), when timesInWord() is more generalized?

timesInWord() has the side-effect of setting incorrectGuesses, which is surprising since it's not obvious from the function's name. Rather than changing the function name, I would recommend adhering to the Single Responsibility Principle: each function should do just one thing.

isNumber() is an odd validation routine. What about punctuation, for example?

Comments

I see that just about every line is commented, which is excessive and annoying. Especially in a language like Python, which some programmers describe as "executable pseudocode", you should be able to write code in a way that speaks for itself. If you do write comments, do so strategically in a way that does not litter your code.

\$\begingroup\$Thank You, on the isInWord I'm not entirely sure why i made it. I was following a challenge sheet my teacher gave to me and it said that I needed to use the function. In hindsight I would probably spend more time on designing a plan, is that something I should do?\$\endgroup\$
– Everless Drop 41Aug 3 '14 at 16:17