My first hangman game

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(" | ")

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.

StackExchange Code Review Q#128063, answer score: 2