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

Hangman game written in Python 3.5

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

Problem

This collects user's guesses for the word. The user has 8 guesses, a correct guess and a repeated guess does not affect the guess count.

```
def getAvailableLetters(lettersGuessed):
'''
lettersGuessed: list, what letters have been guessed so far
returns: string, comprised of letters that represents what letters have not
yet been guessed.
'''
import string
fullstring = string.ascii_lowercase
lettersLeft = ''
for letter in fullstring:
if letter not in lettersGuessed:
lettersLeft = lettersLeft + letter
return lettersLeft

def getGuessedWord(secretWord, lettersGuessed):
'''
secretWord: string, the word the user is guessing
lettersGuessed: list, what letters have been guessed so far
returns: string, comprised of letters and underscores that represents
what letters in secretWord have been guessed so far.
'''
wordGuessed = ''
for letter in secretWord:
if letter in lettersGuessed:
wordGuessed = wordGuessed + letter
else:
wordGuessed = wordGuessed + '_ '
return wordGuessed

def isWordGuessed(secretWord, lettersGuessed):
'''
secretWord: string, the word the user is guessing
lettersGuessed: list, what letters have been guessed so far
returns: boolean, True if all the letters of secretWord are in lettersGuessed;
False otherwise
'''
numCorrect = 0
for letter in secretWord:
if letter in lettersGuessed:
numCorrect += 1
else:
return False
return True

def hangman(secretWord):
guessesLeft = 8
lettersGuessed =[]

print('Welcome to the game Hangman!')
print('I am thinking of a word that is ' + str(len(secretWord)) + ' letters long.' )
print('-----------')

while guessesLeft > 0:
if isWordGuessed(secretWord, lettersGuessed):
return print('Congratulations, you won!')
print('You have ' + str(guessesLeft) + ' guesses left.

Solution

Multiple PEP8 Style Violations

There's a bunch of style violations here, and it really irks me. You are also violating fundamental parts of Python syntax with some of these things, especially with indentation, so you should try and correct style issues as well as my other concerns/suggestions. I try not harping on style, but there's enough issues here that it irks me enough to put this first.

  • import statements must be at the beginning of the file.



  • Function definitions need to be padded by two line breaks. That is, from the end fo one function to the start of the next, you need two blank lines between them (except inside a Class definition, which you don't have here).



  • Function names should be all lowercase, and multiple-word names for functions should be lowercase and separated with underscores (getGuessedWord should be get_guessed_word, etc.)



  • Variable names should be all lowercase, not CamelCase. (The exception are module-wide constants, which should be in all CAPS.). Multi-word variable names (such as lettersGuessed) should be lowercase and separated with underscores (letters_guessed)



  • Docstrings should be triple-quoted, not triple-apostrophe'd. (""", not ''')



  • You have indentation problems that make your code error out in certain cases under hangman where you will have unexpected indentation. (note that I suppress these when testing code here for review, but it's a critical issue). Use 100% consistent indentation - this should be 4 spaces per indentation level, not tabs, and you should not have some lines indented where they are not supposed to be.



User input can be condensed to one line

You currently use this for getting user input:

user_input = input('Please guess a letter: ')
user_input = str(user_input)
user_input = user_input.lower()


This can be condensed into one line, and we can also trim off the line break at the end too (so later recommendations for validation work without much issue):

user_input = input('Please guess a letter: ').lower().strip('\r\n')


No checking for valid inputs

You are expecting only letters to be provided, correct? That's the basis of Hangman.

We should probably test to see if we're actually getting letters provided to us, and make a warning. So, let's do this right after the user enters input:

user_input = input('Please guess a letter: ')).lower().strip('\r\n')
while user_input not in string.ascii_lowercase:
    print("You have not provided a letter, please only use letters!")
    user_input = input('Please guess a letter: ').lower().strip('\r\n')


Output messages: Code repetition

You've got this in here three times:

print('-----------')


You can instead use only one of these, and place it after the if/else block, and thereby not have to repeat this three times in a row inside the if block (note that I've snipped out all other contents of the if/else block, to shorten this post slightly).

if user_input not in letters_guessed:
    ....
else:
    ....

print('-----------')


Use augmented assignment statements.

This is currently in your code in getGuessedWord:

if letter in letters_guessed:
    word_guessed = word_guessed + letter
else:
    word_guessed = word_guessed + '_ '


Two things here: both of these can be condensed using augmented assignment statements, and you may wish to just use underscores without extra spaces between characters in the word, because it looks messy when I have some letters guessed and others not guessed (inconsistent spacing in output):

if letter in letters_guessed:
    word_guessed += letter
else:
    word_guessed += '_'


Use if __name__ == "__main__": to define what to execute when script is called from command line

python3 hangman.py will call hangman("stackoverflow"), but this is not really a good way to achieve it.

We should primarily define something like this at the end of the program to make sure that what runs is what we want to run when this is executed at the command line:

if __name__ == "__main__":
    hangman('stackoverflow')


Note though that you are always going to have "stackoverflow" as the string; you should strongly consider providing a file with valid words for usage, and then random selection therein rather than using a static string as an assignment.


This has an added benefit of 'guarding' the call to hangman() (which is the replacement for a call to main() as some would have) if, for some reason, I was importing parts of the module and importing into another one. This is more or less, also, a personal preference to guard whether I plan on importing or not, it's something I do regardless, just to protect calls to 'start' the program/script.

Unnecessary creation of variables

In get_available_letters, you do this:

fullstring = string.ascii_lowercase
....
for letter in fullstring:


Why do we create fullstring here? It's not needed - we can just use this instead and not h

Code Snippets

user_input = input('Please guess a letter: ')
user_input = str(user_input)
user_input = user_input.lower()
user_input = input('Please guess a letter: ').lower().strip('\r\n')
user_input = input('Please guess a letter: ')).lower().strip('\r\n')
while user_input not in string.ascii_lowercase:
    print("You have not provided a letter, please only use letters!")
    user_input = input('Please guess a letter: ').lower().strip('\r\n')
print('-----------')
if user_input not in letters_guessed:
    ....
else:
    ....

print('-----------')

Context

StackExchange Code Review Q#143552, answer score: 5

Revisions (0)

No revisions yet.