HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python Hangman Console

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
consolepythonhangman

Problem

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 +

Solution

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.

Code Snippets

with open('words.txt') as f:
    words = f.read().splitlines()
#check if the letter is in the word
if letter in word:
    #if so return true
    return True
else:
    #Otherwise return false
    return False
>>> Counter('abracadabra')['a']
5

Context

StackExchange Code Review Q#58899, answer score: 6

Revisions (0)

No revisions yet.