3
\$\begingroup\$

I created a basic Hangman game that uses a text file to select a secret word. Are there any areas for improvement?

import random


secret_word = ['']
user_list = []
number_of_tries = 5
guessed_letters = []
user_tries = 0
user_guess = ''


def select_word():
    global secret_word, user_list
    with open('secret_words.txt', 'r') as f:
        word = f.read()
        word_list = word.split('\n')
        secret_word = word_list[random.randint(1, len(word_list))]
        user_list = ['-'] * len(secret_word)


def game_over():
    if user_tries == number_of_tries or user_list == list(secret_word):
        return True
    else:
        return False


def user_input():
    global user_guess
    user_guess = input('Guess a letter\n')
    check_guess(user_guess)


def repeated(guess):
    global guessed_letters
    if guess in guessed_letters:
        print('You already guessed that letter!\n')
        return True
    else:
        guessed_letters.append(user_guess)
        return False


def check_guess(guess):
    correct_guess = False
    for x in range(len(secret_word)):
        if guess == secret_word[x]:
            user_list[x] = guess
            correct_guess = True
        elif not correct_guess and x == len(secret_word)-1:
            global user_tries
            user_tries += 1
            print('Wrong guess, you lose one try\n'
                  'Remaining tries : {}\n'.format(number_of_tries - user_tries))
    if correct_guess:
        print('Correct guess!')


def valid_input(user_letter):
    valid_letters = 'qwertyuiopasdfghjklzxcvbnm'
    if user_letter.lower() in list(valid_letters):
        return True
    else:
        print('Invalid input')
        return False

# main code:


print('----HANG MAN----')
print('*Welcome, guess the word\n*you have 5 tries.')
select_word()

while not game_over():
    for x in user_list:
        print(x, end='')
    user_guess = input('\nGuess a letter : ')
    if valid_input(user_guess):
        if repeated(user_guess):
            continue
        else:
            check_guess(user_guess)


if user_list != list(secret_word):
    print('Game over, you died!\ncorrect word was {}'.format(secret_word))
else:
    print('Congratulations! you guessed the correct word\n')
\$\endgroup\$
2
\$\begingroup\$

For the word selection, there is a bug on

secret_word = word_list[random.randint(1, len(word_list))]

you should change to

secret_word = word_list[random.randint(0, len(word_list)-1)]

because random.randint(1, len(word_list)) does not return 0 index, and could return an index off the bound (len(word_list)).

Also, you may remove secret_word = [''] and user_list=[] at the beginning.

number_of_tries = 5
guessed_letters = []
user_tries = 0
user_guess = ''

def select_word():
    with open('secret_words.txt', 'r') as f:
        word = f.read()
        word_list = word.split('\n')
        secret_word = word_list[random.randint(0, len(word_list)-1)]
        user_list = ['-'] * len(secret_word)
    return secret_word, user_list

looks more compact. So you can use it as :

print('----HANG MAN----')
print('*Welcome, guess the word\n*you have 5 tries.')

secret_word, user_list = select_word()

...


Also for efficiency and compactness, you can change this

while not game_over():
    for x in user_list:
        print(x, end='')
    user_guess = input('\nGuess a letter : ')
    if valid_input(user_guess):
        if repeated(user_guess):
            continue
        else:
            check_guess(user_guess)

to:

while not game_over():
    print(''.join(user_list))
    user_guess = input('\nGuess a letter : ')
    if valid_input(user_guess):
        if not repeated(user_guess):
            check_guess(user_guess)


For the game itself, you may want to try using classes, which will make it more readable and easier to analyze.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you, I think you explained pjz suggestion of passing variables around instead of using global? will use that looks much better. \$\endgroup\$ – AMJ Jan 18 at 20:39
  • 1
    \$\begingroup\$ As pjz said : "put function results into variables and pass them around," is like what I did the select_word function..we can just move the global variables to become variables with values from results of the function \$\endgroup\$ – Arief Anbiya Jan 19 at 0:09
6
\$\begingroup\$
  • this if statement is a bit silly:

        if user_tries == number_of_tries or user_list == list(secret_word):
            return True
        else:
            return False
    

    It could easily be:

        return user_tries == number_of_tries or user_list == list(secret_word)
    

    since that expression evaluates to True or False, just return it directly

  • Instead of

    secret_word = word_list[random.randint(1, len(word_list))]
    

    you can use the much more readable random.choice:

    secret_word = random.choice(word_list)
    
  • your user_input routine isn't used at all. Nuke it or use it.

  • You use a lot of global state. This is generally frowned upon because it makes the code less reusable. You should first off, try to use less global state: put function results into variables and pass them around, instead. This makes those functions much more reusable. If you absolute must use global state, still don't: instead make an object that stores that "global" state, and turn the functions that use it into methods on the object.

\$\endgroup\$
  • \$\begingroup\$ Thank you for useful comment. Can you explain more about passing variables around \$\endgroup\$ – AMJ Jan 18 at 20:35

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.