4
\$\begingroup\$

I just started learning programming in Python and decided to write a game of hangman to try and learn more about how certain things worked. Any constructive criticism would be greatly appreciated.

import random
print('let\'s play hangman')
#set words
first = ['t','e','s','t']
second = ['t', 'e', 's', 't', 'e', 'd']
third = ['t', 'e', 's', 't', 'i', 'n', 'g']
fourth = ['r', 'a', 'n', 'd', 'o', 'm']
fifth = ['g',  'e',  'n',  'e',  'r',  'a',  't',  'e']
#set list of possible answers
alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 
'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
#set a couple of varibles
answer = []
end = 0
right = 0
wrong = 0
endCheck = 0
alreadyGuessed = 0
#randomly pick a word
random=random.randint(1, 5)
if random == 1: words = first
if random == 2: words = second
if random == 3: words = third
if random == 4: words = fourth
if random == 5: words = fifth
#set varible length to the number of letters in the word
length = len(words)
#figure out what number must be reached in order to declare winner
for loop in range(length):
    end = end + loop
#set answer to as many asterisks as there are letters in the word
for marker in range(length):
    answer.append('*')
#start the main loop and end if win or lose
while wrong < 5 and endCheck != end:
    #print the answer withe asterisks in place of unknown letters
    print ('The word is ', end = '')
    for word in range(length):
        print(answer[word], '', end='')
    print('')
    #start loop to get answer and check if already guessed
    while alreadyGuessed < 1:
        #get guess
        letter = str(input('What is your guess? '))
        #make sure guess is in lowercase
        letter = str.lower(letter)
        #convert guess into ascii number and set to 0 for a, 1 for b...
        guess = ord(letter) - 97
        #check if letter already guessed
        if alphabet[guess] == ['*']:
            print('You already guessed that letter, try again')
            alreadyGuessed = 0
        #otherwise set letter to guessed and continue
        else: 
            alphabet[guess] = ['*']
            alreadyGuessed = 1
    #reset guessed tracker
    alreadyGuessed = 0
    #if guess is right enter it into answer at correct spot and mark this 
    turn as a right guess
    for marker2 in range(length):
        if words[marker2] == letter:
            answer[marker2] = letter
            endCheck = endCheck + marker2
            right = right + 1
    #check if entire word has been guessed
    if endCheck == end:
        print('You guessed it right before getting a hangman! You win!')
        print ('The word was ', end = '')
        for word in range(length):
            print(answer[word], end='')
    #check if a right letter was guessed this turn
    if right > 0 and endCheck != end:
            print('You got one right! no hangman for you this turn')
    #check if a wrong letter was guessed this turn
    if right == 0:
        print('You missed it this turn!')
        wrong = wrong + 1
        print ('number missed =',  wrong)
    #check if to many wrong guesses were made
    if wrong == 5:
        print('You got a hangman!')
    #reset right guesses to 0 before next turn
    right = 0
    #skip a space between turns
    print ('')
\$\endgroup\$
3
\$\begingroup\$

First, some comments on the UI:

The word is t e s t * n *
What is your guess? g
You got one right! no hangman for you this turn

The word is t e s t * n g
What is your guess? i
You guessed it right before getting a hangman! You win!
The word was testing

You need to do a better job separating the "game board" from the verbiage. Also, maybe put some quotes and other punctuation around the printed answer:

The word was, "testing"!

Now for the code. I'm not going to do a complete review because ... it makes my eyes burn. Instead, I'm going to assume that:

  • You're coming to python from C or C# or Java - some language with array support and lots of curly braces.
  • You're in a course, and being deliberately held back.

So let's look at your coding style:

import random
print('let\'s play hangman')
#set words
first = ['t','e','s','t']
second = ['t', 'e', 's', 't', 'e', 'd']
third = ['t', 'e', 's', 't', 'i', 'n', 'g']
fourth = ['r', 'a', 'n', 'd', 'o', 'm']
fifth = ['g',  'e',  'n',  'e',  'r',  'a',  't',  'e']
#set list of possible answers
alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 
'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
#set a couple of varibles

Use vertical space to separate unrelated things

You didn't separate things using vertical space. This is perhaps the greatest sin in programming. Always, always use vertical space (blank lines, paragraphs, etc.) to separate parts of your code. If you switch from doing one thing to another, insert a blank line. If you end a paragraph and start a new paragraph, insert a blank line. If you come to the end of a function or a loop, insert a blank line.

Even if you are the most secretive hacker ever, and you never intend to share your code with a single other living human being, at some point you will have to go back and read stuff that you, yourself wrote six months earlier! Make it easy on yourself, or on whatever other humans have to read your code: try to make it easy to read! Use vertical space to separate unrelated things.

Keep related things together

Next, consider the line:

print('let\'s play hangman')

Ignoring the lack of a leading capital letter, and no ending punctuation, why is this line where it is?

This is clearly the "start of the game." That's fine. But at line #2, you aren't ready to start the game. There's a bunch of variable initialization and other things that need to happen before the game starts. This line should be moved down to where all the rest of the user-facing code is located. Keep related things together.

Let your code speak for itself

Your comments suck.

There are six basic questions you can answer: Who, What, Where, When, Why, and How?

Of those, three are only used in intro-to-programming courses: Who, Where, When. If you have to put a big header block at the top of your program with your student id, name, course number, professor's name, etc., that's okay. Just do it once, and we will all skip right over it!

The other three- What, Why, and How- should only be used if you can't actually answer the question with the code itself!

Specifically, if you can't choose your variable, type, and subroutine names in such a way as to make the answer obvious.

If you can make the answer obvious, then don't add a comment. It just wastes my time, and annoys the compiler.

To wit:

#set varible length to the number of letters in the word
length = len(words)

First, you misspelled "variable." But second: how could you possibly think this comment adds value to your source code?

There are some occasions when you need to answer the question "What?" But those occasions are few and far between. You're actually better off never answering that question unless somebody calls you on it. Until and unless someone asks you "what is this code doing?" I would suggest you simply never use a comment to explain what you are doing - because at the "learning" stage, it is almost always obvious.

You might explain "why" you're computing some value, or "how" you're doing a computation. But, again, for the most part these things are obvious.

Now consider your variable, words:

first = ['t','e','s','t']

# ...
random=random.randint(1, 5)
if random == 1: words = first

Apparently, words is actually the secret word the player is trying to guess. Which is fine, except the whole program is about words, and words is plural - suggesting perhaps a list of words or a dictionary of words or a disk file containing more than one actual word!

This variable is the most important piece of data in your program, and you named it wrong. Maybe secret_word instead? (Because answer is taken.)

There are very few beginner programs that need comments. But there are often programs that need better variable or function names. Let your code speak for itself.

K.I.S.S.

Consider this:

for loop in range(length):
    end = end + loop

# ...

while wrong < 5 and endCheck != end:

    # ...

    for marker2 in range(length):
        if words[marker2] == letter:
            # ...
            endCheck = endCheck + marker2

This is "interesting" because it appears that you determine that the player has won the game by adding up the index of all the correctly guessed letters, 0 + 1 + 2 + 3 + 4 ... and matching the sum against a pre-computed value.

This is so bizarre that I wonder if you copied your solution from a different language. Perhaps in C, or Pascal, or assembly, or just any language with bad string support? Also, of course, it won't work: you start with zero, and you use 0 as the index of the first character. This is a bug.

The word is * e n e r a * e
What is your guess? t
You guessed it right before getting a hangman! You win!
The word was *enerate

Ignoring the whole "it doesn't work" thing, this solution is vulnerable to all sorts of programmer errors, and it is not at all obvious what you're doing.

A far better test for "done" would be simple equality: is A equal to B? It turns out that Python implements list equality correctly. You can simply say,

if listA == listB:

and get the correct result. Alternatively, you could store your words as strings rather than lists. This would make it somewhat awkward to manipulate them, since Python strings are immutable. But it would again let you simply say,

if wordA == wordB:

or even,

while wordA != wordB:

and have a simple, obvious operation for checking if the player has won. As the saying goes, "keep it simple stupid."

A place for everything...

Most of your code is inside a while loop. But much of your code doesn't get executed more than once!

Consider this:

#check if entire word has been guessed
if endCheck == end:
    print('You guessed it right before getting a hangman! You win!')
    print ('The word was ', end = '')
    for word in range(length):
        print(answer[word], end='')
#check if a right letter was guessed this turn
if right > 0 and endCheck != end:
        print('You got one right! no hangman for you this turn')
#check if a wrong letter was guessed this turn
if right == 0:
    print('You missed it this turn!')
    wrong = wrong + 1
    print ('number missed =',  wrong)
#check if to many wrong guesses were made
if wrong == 5:
    print('You got a hangman!')

That code is a mixture of "things to do at the end of the game" and "things to do in a loop."

Mixing things like that is a mistake. First, because it requires you to check your conditions over and over (which is inefficient). And second because you are flipping back and forth: he's in! he's out! he's in! he's out! This is unnecessary complexity (violating the KISS principle), and it doesn't keep related things together.

Instead of jumbling all these conditions together, ask yourself: what things go inside the loop, and what things go outside the loop?

Obviously, winning or losing the game is "outside the loop" because in either case you won't be asking for any more guesses.

Similarly, reporting if a single guess was right or wrong is inside the loop, because then you will ask for more guesses.

Consider using the break or the continue statement to short-circuit the rest of the loop body. Like this:

while True: 
    # guess, etc.
    if answer == secret_word or wrong_guesses == 5:
        break

if answer == secret_word:
    # you win
else:
    # you lose

Identify the various conditions, and then have "A place for everything, and everything in its place!"

I'm not going to address the places where your code is not "Pythonic". You've said you're learning, and I believe you. Plus, I expect some other people will chime in for that. There are a lot of things you can do to clean this code up, and to make better use of language features. But I think you should address the "bigger" issues first, because they will still be true even if you aren't programming in Python. Bad comments in C# are still bad comments. Poor variable names in Scheme are still poor variable names.

\$\endgroup\$
2
\$\begingroup\$

Try to think about how you can combine things to simplify your code.

For example, this:

random=random.randint(1, 5)
if random == 1: words = first
if random == 2: words = second
if random == 3: words = third
if random == 4: words = fourth
if random == 5: words = fifth

can be turned into this:

something = random.choice([first, second, third, fourth, fifth])

And why do you do this?

random=random.randint(1, 5)

Try running random.choice again later in the code.

Also, you might want to refactor your code while keeping in mind to treat strings as immutable lists.

\$\endgroup\$
0
\$\begingroup\$

Judging by the code you have written you may not know about classes or at least are unsure how to write one. My answer will be a rewrite of your program to show the benefits of writing a program in a class. This may serve to get you started with class and also provide a bit of understanding on writing DRY code.

DRY = Don't repeat yourself. Basically anywhere you need to do something multiple times try to create a loop to do the work for you. And do you best to avoid doing the same operation over and over if they are not necessary.

The first thing we need to do is create our class and the line that calls the class.

class HangManGame:
    def __init__(self):

HangManGame()

Next we need to create some class attributes. Class attributes are like variables but with the added bonus of being usable from anywhere inside the class. This means you do not need to use global and makes things go a lot smother when writing your code.

To create a class attribute we write down a variable inside the __init__ portion of our class and you will use the prefix self. to tell python that the variable is a class attribute.

class HangManGame:
    def __init__(self):
        self.available_words = ["test", "tested", "testing", "random", "generate"]
        self.alphabet = "abcdefghijklmnopqrstuvwxyz"

        self.current_word = []
        self.guesses = []
        self.answer = []
        self.letter = ""
        self.bad_guess_counter = 0

        self.select_random_word_from_list()
        print("let's play hangman\n")

Next we can write a few methods.

Methods are class functions. They the great part about class methods is they do not need to be written in any particular order. The only major requirements are to write them after the def __init__(self): method and to make sure they have the argument self. The self argument is very important and is used to interact with the rest of the class attributes and methods.

Below are the 4 methods used for the program.

This first method is used to select a new word for the hangman game. You will notice the use of self. inside the method. This tells the method it needs to look at a class attribute:

def select_random_word_from_list(self):
    self.current_word = [] # clears current word for new game.
    self.guesses = [] # clears the users guesses.
    self.answer = [] # clears the answer to be reset with *
    word = choice(self.available_words) # selects a random word from our list
    for i in word: # for each character in selected string
        self.current_word.append(i)
    for i in word:
        self.answer.append("*")
    self.new_guess() # starts the guessing

This method is called on for each new guess:

def new_guess(self):
    if self.bad_guess_counter == 5: # check if the user can guess again
        print("You lose!")
        self.new_game_check()
    else:
        answ = ""
        for i in self.answer:
            answ += i
        print(answ) # this is used to print how much of the answer has been revealed.
        if "*" in self.answer: # if there are any * in the answer then we know hangman is not complete yet.
            self.letter = input("\nWhat is your guess? ").lower() # updates the users guess for later use.
            self.process_guess() # calls the method that checks the users guess
        else: # if no * are in the answer then the word was guessed!
            print("\nYOU WIN!")

The 3rd method will check the users guess and either tell the user to try again or it will update the answer list with the revealed letters:

def process_guess(self):
    if self.letter in self.alphabet and self.letter != "":
        if self.letter in self.guesses:# checks to see if letter was guessed already
            print("You already guessed that letter, try again")
            self.bad_guess_counter += 1
            self.new_guess()
        else: # checks the current word and updates the index on the answer if it matches
            self.guesses.append(self.letter)
            for i in range(len(self.current_word)):
                if self.letter == self.current_word[i]:
                    self.answer[i] = self.letter
            self.new_guess()
    else:
        print("\nPlease input a letter")
        self.new_guess()

The last method is used to check if the user wants to play again:

def new_game_check(self):
    again = input("Would you like to play again? yes or no?").lower()
    if again == "yes":
        self.select_random_word_from_list()
    else:
        print("Good bye!")

The result is a program that can be played over and over without having to rerun the program. This code is only 20 lines shorter than your code but is much easier to maintain and add to. You could even import a list of possible words from a text file and all you would need to do is add a with open statement that imports each word in the file to a list and you have a hangman game you can play for hours!

Most if not all of the below can be done outside of a class however things get a bit complicated with using global variables and trying to place the functions in an order that the program will accept. With a class this is not an issue.

In the end the code could look something like this:

from random import choice # import choice instead of randint. This will select a random word from a list of words.

class HangManGame:
    def __init__(self):
        self.available_words = ["test", "tested", "testing", "random", "generate"]
        self.alphabet = "abcdefghijklmnopqrstuvwxyz"

        self.current_word = []
        self.guesses = []
        self.answer = []
        self.letter = ""
        self.bad_guess_counter = 0

        self.select_random_word_from_list()
        print("let's play hangman\n")

    def select_random_word_from_list(self):
        self.current_word = []
        self.guesses = []
        self.answer = []
        word = choice(self.available_words)
        for i in word:
            self.current_word.append(i)
        for i in word:
            self.answer.append("*")
        self.new_guess()

    def new_guess(self):
        if self.bad_guess_counter == 5:
            print("You lose!")
            self.new_game_check()
        else:  
            answ = ""
            for i in self.answer:
                answ += i
            print("\n{}".format(answ))
            if "*" in self.answer:
                self.letter = input("\nWhat is your guess? ").lower()
                self.process_guess()
            else:
                print("\nYOU WIN!")
                self.new_game_check()

    def process_guess(self):
        if self.letter in self.alphabet and self.letter != "":
            if self.letter in self.guesses:
                print("You already guessed that letter, try again")
                self.bad_guess_counter += 1
                self.new_guess()
            else:
                self.guesses.append(self.letter)
                for i in range(len(self.current_word)):
                    if self.letter == self.current_word[i]:
                        self.answer[i] = self.letter
                self.new_guess()
        else:
            print("\nPlease input a letter")
            self.new_guess()

    def new_game_check(self):
        again = input("Would you like to play again? yes or no?").lower()
        if again == "yes":
            self.select_random_word_from_list()
        else:
            print("Good bye!")

HangManGame()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.