6
\$\begingroup\$

I made a smart Hangman game in Python and now want your opinion about the readability and code quality of this script. I want to improve my overall coding style. So I am happy to hear improvements for naming conventions, proper docstrings and logical/runtime enhancements.

So lets start with a quick explanation how my program works.

  • The user chooses a word from a dictionary
  • The program gets the possible words from the list based on the length
  • The program calculates the character frequency before asking
  • If the asked character is not in the secret word the possible word list gets updated again
  • If the asked character is in the list the program asks for indices and based on that input the word list gets updated again

This concept works very good. The only problem is if several characters occurs with the same count the program asks in alphabetically order, but this can't be optimized I guess and is acceptable.

Let ́s get through the functions.

Here I load the vocabulary file:

def read_vocabuary_file(path):
 """
 Function to read the vocabulary file
 :param path: Path to file
 :return: List with words
 """
 words = []
 with open(path) as file:
 for line in file:
 words.append(line.rstrip().lower())
 return words

This function checks if the word is in the list

def check_word(word, words):
 """
 Helper function to check if choosen word is in list
 :param word: Word choosed by user
 :param words: Word list
 :return: True if word is in list - False if not
 """
 if word in words:
 return True
 else:
 return False

Get the character frequency

def get_char_frequency(words):
 """
 Function to get the frequency of each character in the word list
 :param words: Word list
 :return: Frequency dict
 """
 char_counts = Counter(letter for word in words for letter in word)
 return char_counts

Function to get words with equal length as the hidden word

def get_equal_words(word_length, words):
 """
 Function to get possible words with same length
 :param word_length: The length of the user word
 :param words: List of words
 :return: Possible words with equal length
 """
 possible_words = [word for word in words if len(word) == word_length]
 return possible_words

Update function to remove words with wrong character

def filter_wrong_characters(character, words):
 """
 Function to update the word list and remove words where asked character is not in
 :param character: The character for that the program asked
 :param words: Current word list
 :return: Updated word list
 """
 possible_words = [word for word in words if character not in word]
 return possible_words

Function to get words where correct character is at a given index

def filter_correct_characters(character, indices, words):
 """
 Function to update the word list with potential words
 :param character: The character for that the program asked
 :param indices: The indicies where the characters are
 :param words: Current word list
 :return: Updated word list
 """
 possible_words = []
 for idx in indices:
 possible_words += [word for word in words if word[int(idx)] == character]
 return list(set(possible_words))

Helper function to print the current game state

def print_screen(your_word, wrong_guess):
 """
 Helper function to print the current game state
 :param your_word: The masked user word
 :return: None
 """
 print(HANGMAN[wrong_guess])
 print(' '.join(str(i) for i in your_word))
 for i in range(len(your_word)):
 print(i, end=" ")
 print()

Helper function to clear the screen for better readability

def clear_screen():
 """
 Helper function to clear terminal
 :return: None
 """
 os.system('cls' if os.name == 'nt' else 'clear')

Finally here is the full source code

from collections import Counter
import os
HANGMAN = ['''
 +---+
 | |
 |
 |
 |
 |
=========''', '''
 +---+
 | |
 O |
 |
 |
 |
=========''', '''
 +---+
 | |
 O |
 | |
 |
 |
=========''', '''
 +---+
 | |
 O |
 /| |
 |
 |
=========''', '''
 +---+
 | |
 O |
 /|\ |
 |
 |
=========''', '''
 +---+
 | |
 O |
 /|\ |
 / |
 |
=========''', '''
 +---+
 | |
 O |
 /|\ |
 / \ |
 |
=========''']
def read_vocabuary_file(path):
 """
 Function to read the vocabulary file
 :param path: Path to file
 :return: List with words
 """
 words = []
 with open(path) as file:
 for line in file:
 words.append(line.rstrip().lower())
 return words
def check_word(word, words):
 """
 Helper function to check if choosen word is in list
 :param word: Word choosed by user
 :param words: Word list
 :return: True if word is in list - False if not
 """
 if word in words:
 return True
 else:
 return False
def get_char_frequency(words):
 """
 Function to get the frequency of each character in the word list
 :param words: Word list
 :return: Frequency dict
 """
 char_counts = Counter(letter for word in words for letter in word)
 return char_counts
def get_equal_words(word_length, words):
 """
 Function to get possible words with same length
 :param word_length: The length of the user word
 :param words: List of words
 :return: Possible words with equal length
 """
 possible_words = [word for word in words if len(word) == word_length]
 return possible_words
def filter_wrong_characters(character, words):
 """
 Function to update the word list and remove words where asked character is not in
 :param character: The character for that the program asked
 :param words: Current word list
 :return: Updated word list
 """
 possible_words = [word for word in words if character not in word]
 return possible_words
def filter_correct_characters(character, indices, words):
 """
 Function to update the word list with potential words
 :param character: The character for that the program asked
 :param indices: The indicies where the characters are
 :param words: Current word list
 :return: Updated word list
 """
 possible_words = []
 for idx in indices:
 possible_words += [word for word in words if word[int(idx)] == character]
 return list(set(possible_words))
def print_screen(your_word, wrong_guess):
 """
 Helper function to print the current game state
 :param your_word: The masked user word
 :return: None
 """
 print(HANGMAN[wrong_guess])
 print(' '.join(str(i) for i in your_word))
 for i in range(len(your_word)):
 print(i, end=" ")
 print()
def clear_screen():
 """
 Helper function to clear terminal
 :return: None
 """
 os.system('cls' if os.name == 'nt' else 'clear')
if __name__ == '__main__':
 words = read_vocabuary_file('vocabulary.txt')
 word = str(input('Please enter a word which is in the vocabulary: \t'))
 word_in_list = False
 while word_in_list == False:
 if check_word(word, words):
 word_in_list = True
 else:
 word = str(input('Word not in list, please choose another one: \t'))
 words = get_equal_words(len(word), words)
 playing = True
 your_word = ["_"] * len(word)
 wrong_guess = 0
 clear_screen()
 print_screen(your_word, wrong_guess)
 while playing:
 word_counts = get_char_frequency(words)
 for char in ''.join(str(i) for i in your_word):
 if char in word_counts:
 del word_counts[char]
 # try/except block if user messed up the input
 # for example if he said no but the word contains the asked character
 # breaks list of possible words
 try:
 most_common = word_counts.most_common()[0]
 except IndexError:
 print('Ops, something went wrong')
 break
 character_contained = str(input(f'Is there a [{most_common[0][0]}] ? - [Y]ES or [N]O :\t'))
 while character_contained.lower() not in ['yes', 'y', 'no', 'n']:
 print("Only yes/no is allowd")
 character_contained = str(input(f'Is there a [{most_common[0][0]}] ? - [Y]ES or [N]O :\t'))
 if character_contained.lower() in ['yes', 'y']:
 valid_indices = False
 while not valid_indices:
 indices = list(input('At which indices? Please start with 0 and separate them with a space:').split())
 for idx in indices:
 try:
 your_word[int(idx)] = most_common[0][0]
 valid_indices = True
 except:
 print("Something went wrong. Please check your input again")
 words = filter_correct_characters(most_common[0][0], indices, words)
 else:
 words = filter_wrong_characters(most_common[0][0], words)
 wrong_guess += 1
 clear_screen()
 print_screen(your_word, wrong_guess)
 if wrong_guess == 6:
 clear_screen()
 print('GAME OVER')
 print_screen(your_word, wrong_guess)
 playing = False
 if len(words) == 1:
 clear_screen()
 print('I found your word:')
 your_word = words[0]
 print_screen(your_word, wrong_guess)
 playing = False

I can't upload the vocabulary file, you can download it here

This can't ge a bit complicated to understand but I didn't found a better solution.

word_counts = get_char_frequency(words)
for char in ''.join(str(i) for i in your_word):
 if char in word_counts:
 del word_counts[char]

Since I always assign the first element of the frequency dict to the variable most_common I first have to delete the characters which the program already guesses correctly. If not the program asks for a character it already founds over and over again.

pacmaninbw
26.1k13 gold badges47 silver badges114 bronze badges
asked May 26, 2022 at 15:28
\$\endgroup\$
1
  • \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. \$\endgroup\$ Commented May 21 at 11:41

1 Answer 1

6
\$\begingroup\$

Organize programs around functions. The best parts of your current code are the small, well-focused functions and the worst part is the stuff after the main-guard where you deal with user inputs in an ineffective manner. Why is it ineffective? Because you abandoned functions. More details below.

Put all code in functions. Everything after the main-guard should be in a function such as main() or play_hangman().

When feasible, select names that agree with grammar. For example, a count of the number wrong guesses should be named in a plural fashion or in a way that conveys the idea of counting: for example, wrong_guesses or n_wrong are better than the singular wrong_guess. Similarly, a list of hangman ascii-images should be HANGMEN, not HANGMAN.

Avoid names based on ambiguous pronouns. Names like your_word or my_foo are often a bad idea. When writing/reading a computer program, who is the "you" and who is the "me" or "we" or "they"? In the current code, the implication is that "you" refers to the human player. But "we" (you and me) are the ones writing the code and thinking about it: our perspective is that of a coder, not a player. Rather than getting bogged down in referential mysteries like that, aim for either simpler names where appropriate (eg, just word) or for clarifying names (eg, secret_word or masked_word).

If you can return directly from a simple function, do so. In a few functions your code becomes unnecessarily bureaucratic, either by creating an intervening variable that doesn't help with code clarity or by adding unnecessary if-else clauses. Two examples:

# Fine, but needlessly heavy.
def get_char_frequency(words):
 char_counts = Counter(letter for word in words for letter in word)
 return char_counts
# Better: return the Counter directly.
def get_char_frequency(words):
 return Counter(letter for w in words for letter in w)
# The if-else structure isn't helping.
def check_word(word, words):
 if word in words:
 return True
 else:
 return False
# Better: return the bool directly.
# It turns out that we don't need this function, but that's a separate issue.
def check_word(word, words):
 return word in words

Alternative names for the filtering functions. Your word-filtering functions have reasonable implementations, but their names are not particularly clear. None of these names convey an accurate sense of what they do: filter_wrong_characters() and filter_correct_characters() sort of imply that we are filtering characters, but we are actually filtering words; and get_equal_words(), in addition to being named differently than the other two functions, sidesteps the main issue, which is not word-equality but word-size. Here are some alternatives to consider. Some points worth noting: (1) the functions establish a naming convention (filter by X); (2) they establish an argument convention (words first because that's what we are filtering, then letter if any, and then other stuff); and (3) the filter_by_known_letter() implementation seems a bit more direct that your current approach because we drive the logic via an iteration over words, eliminating the need to filter out duplicates.

def filter_by_word_size(words, size):
 return [w for w in words if len(w) == size]
def filter_by_excluded_letter(words, letter):
 return [w for w in words if letter not in w]
def filter_by_known_letter(words, letter, indices):
 return [
 w
 for w in words
 if all(w[i] == letter for i in indices)
 ]

Handle user input with reusable functions. You need to ask the user for input several times, and each operation involves similar annoying details. The way to avoid repetitive code like that is to think about collecting user input in a general way: start a while-true loop; give the user a prompt; get the user's reply; convert the reply to a value; validate the value; return if OK, otherwise ask for input again. That sounds simple enough, but the slightly tricky part is realizing that a general-purpose function for user input needs to receive conversion and validation behavior as arguments -- in other words, its arguments will be functions. Here's one way to do that kind of thing. [As it turns out, your program can rely entirely on the choices approach to validation, but I've left the validate parameter in place here to illustrate the general approach.]

def get_input(prompt, convert = None, validate = None, choices = None):
 # Prepare the conversion and validation functions.
 convert = convert or (lambda x: x)
 if choices:
 validate = lambda x: x in choices
 elif validate is None:
 validate = lambda x: True
 # Get user reply: convert and return if valid.
 while True:
 reply = input(prompt + ': ')
 try:
 value = convert(reply)
 if validate(value):
 return value
 except Exception:
 pass
 prompt = 'Invalid input. Try again'

The user-interaction is too heavy and error-prone. Your program reverses the typical hangman approach where the human guesses and the computer reveals. That's perfectly fine both as a game and as a learning exercise, but it does set up an odd dynamic where the computer is asking the human for answers that the computer already knows -- specifically, whether the letter is in the word and where the letter is located. And since humans can make mistakes, this dynamic forces you to handle exceptions caused by data-entry errors related to the indices. Two simplifications are possible. (1) Don't ask the user whether and where (two questions); instead just ask where (interpret empty reply as nowhere). (2) Compute the expected indices and pass them to get_input() as the only allowed choice, eliminating the possibility of data-entry errors.

Some adjustments to the printing function. It's good that you've centralized your screen printing logic in one place. A few adjustments can simplify the function and its usage. (1) You always clear before printing, so just move the clear_screen() call into print_screen(). (2) Strings are iterable, so you can print both the masked word and its indices via simple print-join operations. (3) When the game is over, you have some awkward code that ends up printing the screen twice, first in the ordinary way and then with a game-over message. You can eliminate that repetition by adding an optional message parameter to the print_screen() function.

def print_screen(masked_word, wrong_guesses, message = None):
 clear_screen()
 print(HANGMEN[wrong_guesses])
 print(' '.join(masked_word))
 print(' '.join(map(str, range(len(masked_word)))))
 if message:
 print(message)

Playing hangman. With those changes in place, the game-playing function can be simplified a fair bit. A few other points worth noting: (1) where it makes sense, outsource logical details to utility functions, such as most_common_letter(); (2) readability in a moderately large function like this can be enhanced with fairly simple comments that organize the code into smaller logical units; (3) to get indices from the user, we need a small data-conversion function; and (4) rather than hardcoding six wrong guesses as a game-over condition, compare the number of wrong guesses to the length of HANGMEN.

def play_hangman():
 # Select word from vocabulary.
 words = read_vocabuary_file('vocabulary.txt')
 word = get_input(
 'Please enter a word which is in the vocabulary',
 choices = words,
 )
 # Filter down to words of that size.
 size = len(word)
 words = filter_by_word_size(words, size)
 masked_word = ['_'] * size
 # Play until solved or failed.
 game_over = None
 wrong_guesses = 0
 print_screen(masked_word, wrong_guesses)
 while not game_over:
 # Guess the most common letter.
 letter = most_common_letter(words, masked_word)
 # Force the poor human to enter the letter locations, if any.
 expected_indices = [i for i, c in enumerate(word) if c == letter]
 indices = get_input(
 f'If the word contains [{letter}], enter its indices',
 convert = parse_ints,
 choices = [expected_indices],
 )
 # Filter words based on reply.
 if indices:
 words = filter_by_known_letter(words, letter, indices)
 for i in indices:
 masked_word[i] = letter
 else:
 words = filter_by_excluded_letter(words, letter)
 wrong_guesses += 1
 # Check for game over.
 if wrong_guesses >= len(HANGMEN):
 game_over = 'GAME OVER!'
 elif len(words) == 1:
 game_over = 'I found your word!'
 masked_word = words[0]
 # Display.
 print_screen(masked_word, wrong_guesses, message = game_over)
def most_common_letter(words, masked_word):
 counts = get_char_frequency(words)
 for c in masked_word:
 counts.pop(c, None)
 return counts.most_common()[0][0]
def parse_ints(reply):
 return sorted(map(int, reply.split()))
answered May 27, 2022 at 19:44
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.