Python Forum
Hangman game, feedback appreciated
Thread Rating:
  • 1 Vote(s) - 5 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Hangman game, feedback appreciated
#1
I've been taking the OpenClassroom Python course and the recent assignment was a Hangman with several specific requests:
- there should be separate files such as main, functions, etc.
- it should handle the case where the file for player scores doesn't exist and create one

I've tried a few things, my code is running (there might be mistakes I didn't spot when trying it, though). However, since just reading the corrected version is not everything and being a beginner I'd really like to know what I've done wrong (or right) and what could be improved in terms of readability, maintainability, general coding practise and so on.

Feedback greatly appreciated.

PS: being new to this forum I couldn't find how to directly upload .py and .txt files, I'll try looking harder for it.


### THE HANGMAN - MAIN ###
import re
import pickle
from random import randrange
from hangmanFunctions import *  #custom module


nextStep = False
while not nextStep:
    usrName = input("Welcome to the Hangman's game, please enter your name:\n")
    print("Your name is {}, correct? Y/N.\n".format(usrName))
    answ = input("- ")
    validAnsw = isLetter(answ)
    if validAnsw and answ == "y":
        nextStep = True
    elif validAnsw and answ == "n":
        print("Okay, let's try again.\n")
    else:
            print("You should enter a single letter. Let's try again.\n")
            
#open pickledDict, if not there, create file with empty dict
try:
    userScoresDict = unpickleObj()
except FileNotFoundError:
    userScoresDict = {}
    pickleObj(userScoresDict)
    
#check if user exists in dict, if not creates entry
usrInDict = keyInDict(usrName, userScoresDict)
if usrInDict:
    print("You've already been playing and your currents score is {}.\n".format(
        userScoresDict[usrName]))
else:
    print("This is the first time you're playing: welcome!\n")
    userScoresDict[usrName] = 0     #adding usr to dict
    
score = userScoresDict[usrName] #creating score var

print("You will be given a word and must guess it in less than 8\
 strokes. Ready? let's start!\n")

#game loop starts #
ctd = True
while ctd:
    secretW = generateWord() #picking random word
    secretW = secretW.strip()

    #creating the current word with "*" for each letter
    i = 1
    currentW = "*"
    while i < (len(secretW)):
        currentW += "*"
        i += 1
    print("\t{}-letter word : {}\n".format((len(secretW)), currentW))

    #guessing part for user
    strokes = 1
    found = False
    while not found and (strokes <= 8):
        usrInput = input("- ")
        if len(usrInput) == len(secretW):
            currentW = usrInput
            currentW = currentW.strip()     #in case there's any extra space
            if currentW == secretW:
                found = True
                print("Congratulations, you win!")
            else:
                print("Sorry, maybe that was too soon.\n")
        elif re.search(usrInput, secretW) is not None: #if anything matches
            currentW = matchingLW(usrInput, currentW, secretW)
            print("{}\n".format(currentW))
            if currentW == secretW:
                found = True
                print("Congratulations, you win!")
        elif re.match(usrInput, secretW) is None:
            print("Nope, sorry")
        if strokes == 8:
            print("Sorry, 8 strokes, game over.\n\The word was {}.".format(secretW))
        strokes += 1
    if (strokes == 8) and not found:
        score -= 5  #losing points for not finding
    else:
        score += (8-strokes)    #handling the score

    #asking if usr wishes to continue playing or not
    gotAnsw = False
    while not gotAnsw:
        print("So, {}, would you like to play again? Y/N\n".format(usrName))
        answ = input("- ")
        answ = answ.casefold()
        if answ == "y":
            ctd = True
            gotAnsw = True
        elif answ == "n":
            ctd = False
            gotAnsw = True
        else:
            print("Sorry, couldn't read your answer, let's try again.\n")
            
#saving current player's score to dict
userScoresDict[usrName] = score
print("Your score is now {}. See you soon.\n".format(userScoresDict[usrName]))
pickleObj(userScoresDict)   #saves current dict to file
### FUNCTIONS FOR HANGMAN GAME ###
import re
from random import randrange
import pickle


def keyInDict(myKey, myDict):
    """
    Checks whether the given key exists in given dict,
    returns True if so, False otherwise
    """
    if myKey in myDict:
            return True
    else:
            return False    


def pickleObj(myObject):
    """
    Stores a serialized objet into a pickled file
    """
    with open("scores", "wb") as fh:    #file is "scores"
        pickle.dump(myObject, fh, protocol=pickle.HIGHEST_PROTOCOL)


def unpickleObj():
    """
    Reads from a pickled file and returns the serialized object
    """
    with open("scores", "rb") as fh:    #file is "scores"
        myUnpickledObj = pickle.load(fh)
    return myUnpickledObj


def isLetter(param):
    """
    Checks the variable given in parameter and determines whether
    it's a signle letter or not, returns True if single letter,
    False otherwise
    """
    param = param.lower() #turning into lower case
    param = param.strip() #trimming
    if (len(param) == 1) and (re.match("[a-z]", param) is not None):
            return True #char is alphabetical and single
    else:
        return False


def generateWord():
    """
    Chooses at random a word from the wordbank given in file
    """
    myList = open("wordList.txt").readlines()   #creating list from file
    word = myList[randrange(len(myList))]   #word picked at random from list
    return word


def matchingLW(letter, currentW, secretW):
    """
    Replaces every matched letter in the current word
    """
    secretWList = list(secretW) #turning string into list
    currentWList = list(currentW)   #turning string into list
    i = 0
    while i < len(secretW):
        if letter == secretWList[i]:
            currentWList[i] = letter
        i += 1
    resultW = "".join(currentWList)   #turning list into string
    return resultW 
### And here the content of the wordList ###
abasedly
abashing
abatable
abatises
abattoir
abbacies
abbatial
abbesses
abdicate
abdomens
abdomina
abducens
abducent
abducing
Reply
#2
quick glance.
1. Top code should be in a function not written in global space.
2. All function names should be lower case. Python style guide
def keyInDict(myKey, myDict):
    """
    Checks whether the given key exists in given dict,
    returns True if so, False otherwise
    """
    if myKey in myDict:
            return True
    else:
            return False  
Can do this
def key_in_dict(my_key, my_dict):
    return my_key in my_dict
99 percent of computer problems exists between chair and keyboard.
Reply
#3
You are way over thinking things. Take these bits of code:

usrInDict = keyInDict(usrName, userScoresDict)
if usrInDict:

...

def keyInDict(myKey, myDict):
    """
    Checks whether the given key exists in given dict,
    returns True if so, False otherwise
    """
    if myKey in myDict:
            return True
    else:
            return False
All of that could be replaced by:

if usrName in userScoresDict:
And these three are equivalent in the context of your code:

re.match("[a-z]", param) is not None
re.match('[a-z]', param)
param.isdigit()
And the last one doesn't require lower casing the text first.

Python is designed to make it easy to write simple code. Take advantage of that. Don't over complicate things.
Craig "Ichabod" O'Brien - xenomind.com
I wish you happiness.
Recommended Tutorials: BBCode, functions, classes, text adventures
Reply
#4
Some random thoughts, there are others too
  • read PEP8 and apply it, e.g. variable and function names are not compliant
  • don't use star import like from hangmanFunctions import *
  • this
    def generateWord():
        """
        Chooses at random a word from the wordbank given in file
        """
        myList = open("wordList.txt").readlines()   #creating list from file
        word = myList[randrange(len(myList))]   #word picked at random from list
        return word
    is better as
    def generateWord():
        """
        Chooses at random a word from the wordbank given in file
        """
        with open("wordList.txt") as f:
            words = [word.strip() for word in f]   #creating list of words from file, strip new line
        return random.choice(words)   #word picked at random from list, without new line at the end
    even better would be if you read all words only once at the beginning of the game, shuffle the list and then pop a word from the end till user wants to play new games. this way there is no chance you would repeat a word

  • this
    i = 1
        currentW = "*"
        while i < (len(secretW)):
            currentW += "*"
            i += 1
    is just
    currentW = '*' * len(secretW)
  • this
    def matchingLW(letter, currentW, secretW):
        """
        Replaces every matched letter in the current word
        """
        secretWList = list(secretW) #turning string into list
        currentWList = list(currentW)   #turning string into list
        i = 0
        while i < len(secretW):
            if letter == secretWList[i]:
                currentWList[i] = letter
            i += 1
        resultW = "".join(currentWList)   #turning list into string
        return resultW 
    is just
    def matchingLW(letter, currentW, secretW):
        """
        Replaces every matched letter in the current word
        """
        result = [secret if secret == letter else current for current, secret in zip(currentW, secretW)]
        return "".join(result)
    matching can be even better, but needs to rewrite bigger part of your script

If you can't explain it to a six year old, you don't understand it yourself, Albert Einstein
How to Ask Questions The Smart Way: link and another link
Create MCV example
Debug small programs

Reply
#5
Thanks a lot for all the input.
I've just become aware of the PEP-8 standards and will comply with the lower case and underscore now.

As for the over complicating things, the bits of code you showed are so much better... I think the fact that the only little knowledge I had of programming before this was a 2 month MOOC on C. I have to change mindsets since Python allows much shorter ways to do things.

@Windspar: what do you mean by "top code"? the import parts?
@buran: why not use star import? I'm just curious about the reason since the OC course I've been following advises to use it when we dont want to do the long calling my_package.my_function() but just my_function call.
Reply
#6
Star imports are problematic because it's not clear what is being imported. Say you import the module foo, which has a function named bar. But you don't realize that, and you write a function named bar. Now your bar function has overwritten foo's bar function. If other functions in foo use foo's bar function, that could cause real problems.

Here it's not really a problem, because they are helper functions and you know what they are. However, it does make the program harder to maintain/expand, because you have to remember not to overwrite those functions. It's generally a good idea to just avoid star imports, and only use them if you are really sure it's a good idea. I would advise getting used to my_package.my_function(), it's a good way to write Python code.
Craig "Ichabod" O'Brien - xenomind.com
I wish you happiness.
Recommended Tutorials: BBCode, functions, classes, text adventures
Reply
#7
(Jul-16-2018, 01:57 PM)ichabod801 Wrote: And these three are equivalent in the context of your code:

re.match("[a-z]", param) is not None
re.match('[a-z]', param)
param.isdigit()

Thanks for the equivalence, I didn't know how to properly write RE. About the last one I thought isdigit looked for digit, numbers and not letters so I'm confused, does it actually look for letters?
Because I had read the following in tutorialpoint:
"This method returns true if all characters in the string are digits and there is at least one character, false otherwise."
and therefore thought tha it was looking for numbers and not letters.
Reply
#8
Quote: @Windspar: what do you mean by "top code"? the import parts?
example
### THE HANGMAN - MAIN ###
import re
import pickle
from random import randrange
from hangmanFunctions import *  #custom module
 
def main():
    nextStep = False
    while not nextStep:
        usrName = input("Welcome to the Hangman's game, please enter your name:\n")
        print("Your name is {}, correct? Y/N.\n".format(usrName))
        answ = input("- ")
        validAnsw = isLetter(answ)
        if validAnsw and answ == "y":
            nextStep = True
        elif validAnsw and answ == "n":
            print("Okay, let's try again.\n")
        else:
                print("You should enter a single letter. Let's try again.\n")
                 
    #open pickledDict, if not there, create file with empty dict
    try:
        userScoresDict = unpickleObj()
    except FileNotFoundError:
        userScoresDict = {}
        pickleObj(userScoresDict)
         
    #check if user exists in dict, if not creates entry
    usrInDict = keyInDict(usrName, userScoresDict)
    if usrInDict:
        print("You've already been playing and your currents score is {}.\n".format(
            userScoresDict[usrName]))
    else:
        print("This is the first time you're playing: welcome!\n")
        userScoresDict[usrName] = 0     #adding usr to dict
         
    score = userScoresDict[usrName] #creating score var
     
    print("You will be given a word and must guess it in less than 8\
     strokes. Ready? let's start!\n")
     
    #game loop starts #
    ctd = True
    while ctd:
        secretW = generateWord() #picking random word
        secretW = secretW.strip()
     
        #creating the current word with "*" for each letter
        i = 1
        currentW = "*"
        while i < (len(secretW)):
            currentW += "*"
            i += 1
        print("\t{}-letter word : {}\n".format((len(secretW)), currentW))
     
        #guessing part for user
        strokes = 1
        found = False
        while not found and (strokes <= 8):
            usrInput = input("- ")
            if len(usrInput) == len(secretW):
                currentW = usrInput
                currentW = currentW.strip()     #in case there's any extra space
                if currentW == secretW:
                    found = True
                    print("Congratulations, you win!")
                else:
                    print("Sorry, maybe that was too soon.\n")
            elif re.search(usrInput, secretW) is not None: #if anything matches
                currentW = matchingLW(usrInput, currentW, secretW)
                print("{}\n".format(currentW))
                if currentW == secretW:
                    found = True
                    print("Congratulations, you win!")
            elif re.match(usrInput, secretW) is None:
                print("Nope, sorry")
            if strokes == 8:
                print("Sorry, 8 strokes, game over.\n\The word was {}.".format(secretW))
            strokes += 1
        if (strokes == 8) and not found:
            score -= 5  #losing points for not finding
        else:
            score += (8-strokes)    #handling the score
     
        #asking if usr wishes to continue playing or not
        gotAnsw = False
        while not gotAnsw:
            print("So, {}, would you like to play again? Y/N\n".format(usrName))
            answ = input("- ")
            answ = answ.casefold()
            if answ == "y":
                ctd = True
                gotAnsw = True
            elif answ == "n":
                ctd = False
                gotAnsw = True
            else:
                print("Sorry, couldn't read your answer, let's try again.\n")
                 
    #saving current player's score to dict
    userScoresDict[usrName] = score
    print("Your score is now {}. See you soon.\n".format(userScoresDict[usrName]))
    pickleObj(userScoresDict)   #saves current dict to file

main()
99 percent of computer problems exists between chair and keyboard.
Reply
#9
(Jul-17-2018, 02:40 AM)WolfWayfarer Wrote: About the last one I thought isdigit looked for digit, numbers and not letters so I'm confused, does it actually look for letters?

No, you're right. I use isdigit a lot to check for integers, but I meant to use isalpha in that example.
Craig "Ichabod" O'Brien - xenomind.com
I wish you happiness.
Recommended Tutorials: BBCode, functions, classes, text adventures
Reply
#10
@Windspar: so there is a main() in Python too! I'm glad, it felt really weird after C just barging in like that. I'm surprised OC doesn't say it, or perhaps I missed that part. Anyhow, thanks alot. It'll definitely look better.

#ichabod801: oh ok, isalpha is helpful, I didn't know it existed and thought I had to use RE.

Thanks for your inputs.
Reply


Possibly Related Threads…
Thread Author Replies Views Last Post
  A hangman game. mcmxl22 1 2,093 Oct-07-2019, 02:02 PM
Last Post: ichabod801
  tk hangman game joe_momma 0 2,861 Aug-09-2019, 02:48 PM
Last Post: joe_momma
  Looking for feedback on a game I made MrPucake 7 4,291 Apr-04-2018, 01:53 PM
Last Post: MIPython

Forum Jump:

User Panel Messages

Announcements
Announcement #1 8/1/2020
Announcement #2 8/2/2020
Announcement #3 8/6/2020