6
\$\begingroup\$

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
\$\endgroup\$

2 Answers 2

Reset to default
6
\$\begingroup\$

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:

#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

Self-documenting code is viewed as good by the Python style guide (known as PEP8) and the Zen of Python even says that:

Explicit is better than implicit.

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...

This piece of code could be improved:

#Make sure it's actually a letter(is not a number)
x = letter
if isNumber(x):
    print ("Invalid")
    return getLetter()

If you want to check whether a one character string is a letter, you better you the method isalpha from the class str:

if not letter.isalpha():
    print("Invalid")
    return getLetter()

Therefore, you can totally get rid of the function isNumber since it is not used anywhere else.

\$\endgroup\$
1
  • \$\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\$ Commented Aug 3, 2014 at 16:13
7
\$\begingroup\$

File input

  • If you want the lowercase alphabet, use string.ascii_lowercase.

  • Calling open() like that, with no corresponding close(), could be a file descriptor leak. You're almost always better off calling open() from a with block:

    with open('words.txt') as f:
    words = f.read().splitlines()
  • To randomly pick an element from an array, use word = random.choice(words).lower().

Python fluency

  • PEP 8, the standard Python style guide, specified that function names should be lower_case.

  • This code in isInWord() is unnecessarily verbose:

    #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.

\$\endgroup\$
1
  • \$\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\$ Commented Aug 3, 2014 at 16:17

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.