3
\$\begingroup\$

I've been trying to learn Python and I've managed to muddle through enough to put together a simple Hangman game (albeit with a few bugs and game features missing!).

However, I feel like I haven't done a very good job as I seem to have repeated a lot of code.

Does anyone have any simple tips for how I could improve the efficiency of the code and stop repeating so much?

Not looking for advanced techniques - just any glaringly obvious basic pitfalls that I should be avoiding in general. (I basically did trial and error until I got it to work so I'm sure there are many coding conventions that I've violated horribly.)

import random


# strips non alpha characters from the random word and converts to upper case
def alpha_only(word):
    valid_word = []
    for char in word:
        if char.isalpha():
            upper_char = char.upper()
            valid_word.append(upper_char)
    return "".join(valid_word)


def artwork(err):
    if err == 1:
        print("-------------")
    elif err == 2:
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 3:
        print("      -------")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 4:
        print("      -------")
        print("      | /    ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 5:
        print("      -------")
        print("      | /   |")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 6:
        print("      -------")
        print("      | /   |")
        print("      |     o")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 7:
        print("      -------")
        print("      | /   |")
        print("      |     o")
        print("      |     O")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 8:
        print("      -------")
        print("      | /   |")
        print("      |     o")
        print("      |    -O-")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 9:
        print("      -------")
        print("      | /   |")
        print("      |     o")
        print("      |    -O-")
        print("      |    / ")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    elif err == 9:
        print("      -------")
        print("      | /   |")
        print("      |     o")
        print("      |    -O-")
        print("      |    / \"")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")
    else:
        print("      -------")
        print("      | /   |")
        print("      |     o")
        print("      |    -O-")
        print("      |    / \\")
        print("      |      ")
        print("      |      ")
        print("      |      ")
        print("-------------")


def guess(ltr, word, lst):
    upper_ltr = ltr.upper()
    if len(upper_ltr) > 1:
        print("That's more than 1 letter, try again.")
        msg = 1

    elif len(upper_ltr) < 1:
        print("You didn't enter a letter, try again:")
        msg = 2

    elif upper_ltr in lst:
        print("You already guessed that letter, try again:")
        msg = 3

    elif upper_ltr not in word:
        msg = 4
    else:
        for n in range(0, len(word)):
            if word[n] == upper_ltr:
                lst[n] = upper_ltr
        msg = 5
    return msg


def game(generated_word):
    hangman_lst = []
    guess_num = 0
    for i in range(0, len(generated_word)):
        hangman_lst.append("__")
    print(" ".join(hangman_lst))
    l = input("This is your first guess. Guess a letter!")

    while "__" in hangman_lst:
        if guess_num == 0:
            flag = guess(l, generated_word, hangman_lst)
            if "__" not in hangman_lst:
                print(" ".join(hangman_lst))
                print("**Ta-da!** You're a winner, go and treat yourself to a peanut butter smoothie.")
                print("")
                print("  o ~ you're my hero")
                print(" -O-")
                print(" / \\")

            elif flag == 5:
                print("Nice job. That's one. This is still your first guess.")
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            elif flag == 1:
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            elif flag == 2:
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            elif flag == 3:
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            else:
                print("NOPE! Sorry, try another letter.")
                artwork(guess_num + 1)
                print(" ".join(hangman_lst))
                guess_num += 1
                l = input("This is guess #" + str(guess_num + 1) + ":")
        elif guess_num == 9:
            print("YOU LOSE :-(")
            print("The word was " + str(random_word_stripped))
            artwork(guess_num + 1)
            break
        else:
            flag = guess(l, generated_word, hangman_lst)
            if "__" not in hangman_lst:
                print(" ".join(hangman_lst))
                print("**Ta-da!** You're a winner, go and treat yourself to a peanut butter smoothie.")
                print("")
                print("  o ~ you're my hero")
                print(" -O-")
                print(" / \\")
            elif flag == 5:
                print("Nice job. That's one. You get a bonus guess.")
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            elif flag == 1:
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            elif flag == 2:
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            elif flag == 3:
                print(" ".join(hangman_lst))
                l = input("This is guess #" + str(guess_num + 1) + ":")
            else:
                print("NOPE! Sorry, try another letter.")
                artwork(guess_num + 1)
                print(" ".join(hangman_lst))
                guess_num += 1
                l = input("This is guess #" + str(guess_num + 1) + ":")


# imports large text file and selects a random word
with open('C:\\big.txt', 'r') as f:
    all_lines = f.readlines()
    random_word_stripped = ""
    while True:
        random_line_str = random.choice(all_lines)
        random_line_lst = random_line_str.split()
        if random_line_lst:
            random_word = random.choice(random_line_lst)
            random_word_stripped = alpha_only(random_word)
            if 5 < len(random_word_stripped) < 9:
                # print(random_word_stripped)
                break

game(random_word_stripped)
\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to Code Review! You mention in your post that your code contains a few bugs. Please note that Code Review is about improving existing, working code. If the code does not behave as expected (i.e. has bugs), it is not ready for review yet. Please clarify this point and see our help center for more information. \$\endgroup\$ – Mathias Ettinger May 11 '16 at 7:32
  • \$\begingroup\$ Hi Mathias, thanks for the welcome! Hmm, I guess I mean there is a bug where if you guess the right letter on the last go then it doesn't register it as a correct guess. But that's the only bug I know of. But the game works ok apart from that. I wasn't too worried about that bug, but just the fact that my code in general is pretty horrible and I was hoping there was a way to use functions more efficiently to prevent me writing some of the content out twice and make it more concise? \$\endgroup\$ – chewflow May 11 '16 at 7:40
  • \$\begingroup\$ Oh, I see you edited my @200_success. Can I ask why it's not ok to say thanks in a post? \$\endgroup\$ – chewflow May 11 '16 at 9:29
  • \$\begingroup\$ It's Stack Exchange policy to remove thanks. \$\endgroup\$ – 200_success May 11 '16 at 9:35
  • \$\begingroup\$ Ah ok, I see, makes sense - cheers! \$\endgroup\$ – chewflow May 11 '16 at 20:40
2
\$\begingroup\$

First things first, use triple quoted strings for your ascii art. You can store all the stages in a list of strings that way, like so:

hangmans = [ 
"-------------",
"""\
    |      
    |      
    |      
    |      
    |      
    |      
    |      
-------------""",
"""\
    -------
    |      
    |      
    |      
    |      
    |      
    |      
    |      
-------------""",
...]

To clarify, by using """ you can spread your string over multiple lines and Python will preserve those newlines. Using \ right before a linebreak means that line is ignored. That's mostly handy for letting each string start at the same depth (otherwise the three double quotes would shift your first line further to the right than the rest:

"""    -------
    |      
    |      
    |      
    |      
    |      
    |      
    |      
-------------""",

And now you can just print from the hangmans list directly with print(hangmans[err]).

In guess, you story ltr as upper_ltr after your conversion. I think this is a bad habit. You have no more need of ltr, so just replace it with the correct form of data. I'd also recommend using either letter or character, rather than the less readable ltr.

Another confusing thing here is that it's not immediately clear what guess is doing. Is the user entering their guess? Is the guess being validated as a potential guess? Are you checking if the guess is a correct letter? It seems like you're doing both validation and checking for correctness but I'm not even sure.

A good way to approach functions is to have each function perform a job. Have a function that validates the user entered a valid guess (only 1 character, and not a previous guess). One function that takes user input, and returns their guess only when they give something valid. Then a separate function to determine whether the guess was right or not. Mixing some of these goals creates confusing and hard to adjust code.

A clear bad pattern is that you will print a message based on the user's input in guess but then you need to return a flag in order to print another message in your big game function. These shouldn't be split, print all the info related to the result of their guess in one go, and then move onto the next guess.

You also repeat big blocks of very similar code. Why is it so important if guess_num == 0? I can't tell from looking at this code, but if you had some sort of process_guess function with much smaller changes based on the guess_num then it'd probably be a lot easier to follow. You need to break down your code into a series of individual tasks, and structure your functions around that. Maybe something like this:

new_game:
    word = find_word

    for round in total_guesses
        get_valid_guess()
        if guess_is_right:
            update_wordstring
            if word_completed
                win_game
                break
    if not win_game:
        print loss_message
    newgame_query

There's obviously parts missing from there, but most of those lines should be their own function, like get_valid_guess, update_wordstring and win_game. Think about the code in broader terms, solve the little problems and then put together the functions to make a much cleaner overall script.

\$\endgroup\$
  • \$\begingroup\$ Hey, thanks a lot for this detailed reply @SuperBiasedMan! I think you've hit the nail on the head - I was struggling to separate out a lot of the actions into distinct functions. I wonder if it's easier if I map these actions out on pen and paper first before I start even coding? Like you said, once I got the code half working I was loathe to go back and get rid of some of the bugs as it was hard to know where to start! Perhaps I'll have another crack from scratch and try to make a clean version... \$\endgroup\$ – chewflow May 11 '16 at 20:46
  • \$\begingroup\$ @chewflow Absolutely. Writing this stuff out on paper is a good way to work through the problems ahead of time. That way you have a plan before you start, rather than building up a plan as you across issues. I'm glad this helped! \$\endgroup\$ – SuperBiasedMan May 12 '16 at 9:22

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.