8
\$\begingroup\$

I decided to code a simple Hangman in Python today and wanted some feedback on the code since I only started programming three weeks ago.

I intentionally didn't add a lot of whitespace, it just doesn't make it any more readable for me - but feel free to give me reasons to start doing so.

Any tips and suggestions are appreciated.

import os

words = ["foobar", "gnarf", "fnord"]

current_word = 0
maximum_words = len(words)
maximum_guesses = 10
solved_words = 0

for word in words:
    current_word += 1
    current_guess = 0
    dashes = []
    for letter in word:
        dashes.append("_")
    while current_guess < maximum_guesses:
        current_guess += 1
        os.system("clear")
        print("Word {}/{}".format(current_word, maximum_words))
        print("Solved {}/{}".format(solved_words, maximum_words))
        print("Guess {}/{}".format(current_guess, maximum_guesses))
        print("\n{}\n".format(" ".join(dashes)))
        guess = input("Guess: ")
        guess = guess.lower()
        if not len(guess) == 1 or not guess.isalpha():
            current_guess -= 1
        for index, letter in enumerate(word):
            if letter == guess:
                dashes[index] = letter 
        if not "_" in dashes:
            solved_words += 1
            break

The output looks something like this:

Word 1/3
Solved 0/3
Guess 5/10

f o o b a _

Guess: r

Any suggestings for a better output?

\$\endgroup\$
  • 1
    \$\begingroup\$ Just a tidbit of advice: you say you don't like whitespace, but adding whitespace as a "logical separator between sections of code" helps reviewers like us to more easily read your code. Also, which Python did you write this for, Python 3? \$\endgroup\$ – Thomas Ward Dec 2 '16 at 14:24
  • \$\begingroup\$ @ThomasWard The code is written in Python 3, should have mentioned above. And you're right, some whitespace would make it indeed a tad more readable. Well, there is always a next time. \$\endgroup\$ – Reum12 Dec 2 '16 at 14:29
  • 1
    \$\begingroup\$ I would suggest adding the python 3 tag then to your question; that way it's more clear. (When just using the 'python' tag, you should also include one of the versioned python tags so we know the target python version) \$\endgroup\$ – Thomas Ward Dec 2 '16 at 14:32
7
\$\begingroup\$

I see a couple issues, though nothing too major.


Improper form for 'if not in'

While this works, it's not the proper form for this:

if not "_" in dashes:

The better form for this is:

if "_" not in dashes:

Tracking if a value is not equal to something

This works:

if not len(guess) == 1 or not guess.isalpha():

But, we can remove one 'not' though, and use an inequality operator instead:

if len(guess) != 1 or not guess.isalpha():

Generation of dashes

You can shorten the creation of 'dashes' to this when you create it, and then get rid of the for loop that generated it previously:

dashes = ['_'] * len(word)

No tracking of guesses already made

Your code does not track guesses already made. Therefore, people can 'waste' guesses.

Let's do two things: first let's move around some things. Secondly, let's add some code to capture this case and not increment the number of guesses made when this happens.

First, let's change where we increment the 'current guess' value, and define a different 'start' value:

for word in words:
    current_word += 1
    current_guess = 1
    dashes = []
    ....
    while current_guess < maximum_guesses:
        os.system("clear")
        ....
        guess = guess.lower()
        if not len(guess) == 1 or not guess.isalpha():
            current_guess += 1
            ....

Basically, we moved the current_guess incrementer to lower in the code. And initialize the guess counter to 1 to begin with.

Now let's do some "already guessed" code:

for word in words:
    ....
    guesses = set()
    for letter in word:
        ...
    guess = guess.lower()
    if guess not in guesses:
        guesses.add(guess)
        current_guess += 1
        if not len(guess) == 1 or not guess.isalpha():
            current_guess -= 1
        for index, letter in enumerate(word):
            if letter == guess:
                dashes[index] = letter
        if "_" not in dashes:
            solved_words += 1
            break

This allows 'repeated' guesses of the same letter to not take effect. That way, if someone enters 'o' as a guess twice, accidentally or intentionally, on the word 'foobar', they aren't penalized for something already guessed. You can choose to not do this, but it helps protect against "What the heck, I shouldn't be penalized for something I already guessed!" complaints.


I don't change much of your program. This is the total program with my above suggestions, plus some whitespace for readability. I don't have any suggestions for improving the output, though, because it seems to be okay as it is, at least to me.

import os

words = ["foobar", "gnarf", "fnord"]

current_word = 0
maximum_words = len(words)
maximum_guesses = 10
solved_words = 0

for word in words:
    current_word += 1
    current_guess = 1
    dashes = ['_'] * len(word)
    guesses = set()

    while current_guess < maximum_guesses:
        os.system("clear")

        print("Word {}/{}".format(current_word, maximum_words))
        print("Solved {}/{}".format(solved_words, maximum_words))
        print("Guess {}/{}".format(current_guess, maximum_guesses))
        print("\n{}\n".format(" ".join(dashes)))

        guess = input("Guess: ")
        guess = guess.lower()

        if guess not in guesses:
            guesses.add(guess)
            current_guess += 1

            if len(guess) != 1 or not guess.isalpha():
                current_guess -= 1

            for index, letter in enumerate(word):
                if letter == guess:
                    dashes[index] = letter

            if "_" not in dashes:
                solved_words += 1
                break
\$\endgroup\$
  • 2
    \$\begingroup\$ 1. change guesses to set so your in lookup will be O(1). 2. instead of building dashes in a loop you can simply do dashes = ['_'] * len(word) \$\endgroup\$ – Alex Dec 2 '16 at 15:00
  • 1
    \$\begingroup\$ You may want to add an else to your if guess not in guesses just to warn the user the guess was already made. And maybe change the order around: if guess in guesses: print('You already tried this letter') else: # rest of the code. \$\endgroup\$ – 301_Moved_Permanently Dec 2 '16 at 15:04
  • \$\begingroup\$ @MathiasEttinger A little hard to do with their terminal "clear" call that runs every iteration of their loop. It'll eat the output. But you're right, that would be a good thing to add. (I'm not going to restructure their program in that way though, right now) \$\endgroup\$ – Thomas Ward Dec 2 '16 at 15:05
  • \$\begingroup\$ @ThomasWard Right, didn't pay enough attention to this \$\endgroup\$ – 301_Moved_Permanently Dec 2 '16 at 15:06
  • \$\begingroup\$ @Alex suggestions incorporated, please let me know if I did it wrong for the set :) \$\endgroup\$ – Thomas Ward Dec 2 '16 at 15:07
1
\$\begingroup\$

You should implement the ability to read words from a txt document. This can be done by:

f = open("wordlist.txt", 'w')
word_list.append(f.readlines())

this would allow you to have far bigger lists to choose from.

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