3
\$\begingroup\$

Recently, I have been trying to improve my functional programming skills and understanding. While at work today, my coworkers and I were playing Hangman in a group chat. I thought of a program that would guess the likeliest letter in the word to guess. So I implemented it when I got home. Here is the repository if you would like any more information (such as the word text file that is required for this script).

I would definitely appreciate comments on the readability of my code, as well as the uses of functional programming. Also, I was thinking that I may have been over commenting in my code. Is that the case?

import re


def get_words(word_len):
    # Open the file with all the words in the English language.
    with open("words.txt") as word_file:
        # Get all the words without any newlines.
        words_temp = map(lambda s: s.strip(), word_file.readlines())
        # filter the words so that they have the same number of characters as the word in play.
        words = [word.lower() for word in words_temp if len(word) is word_len]
        # Get rid of any possible duplicates in the file.
        words = list(set(words))
    return words


def get_possible_words(guesses, current_word):
    # The total number of characters in the word.
    num_of_characters = len(current_word)

    # Load the words in from the words.txt file.
    words = get_words(num_of_characters)

    # Get all words with just letters.
    words = list(filter(lambda w: w.isalpha(), words))

    # Regex will give us an error if we have
    # no wrong guesses, so if we don't need to exclude
    # anything, include everything!
    if len(guesses) is 0:
        substitute = '.'
    else:
        # exclude all of the wrong guesses
        substitute = f"[^{guesses}]"

    # Make the current_word a regex phrase.
    current_word_regex = current_word.replace('_', substitute)

    # Get the regex object for the current word
    regex_obj = re.compile(current_word_regex)

    # Get all possible matches to the word.
    possible_matches = list(map(lambda word: regex_obj.match(word), words))

    # Get all the words from those matches (filter None matches)
    possible_words = [match.string for match in possible_matches if match is not None]

    # Print the list of possible words.
    return possible_words


def get_statistics(possible_words):
    # Join all of the words in the list into a giant string.
    words_as_str = ''.join(possible_words)
    # sort the characters in each word.
    words_as_str = ''.join(sorted(words_as_str))

    # get all of the characters in the words.
    characters_in_words = ''.join(set(words_as_str))

    # Get the frequencies of each letter in the words.
    frequencies = {c: words_as_str.count(c) for c in characters_in_words}

    return frequencies


def get_likeliest_letter(stats):
    # Get the most likely letter to guess.
    likeliest_letter = max(stats, key=stats.get)

    # Get the likelihood of the letter as a percent.
    likelihood = stats[likeliest_letter] / sum(stats.values()) * 100.0

    return likeliest_letter, likelihood


def play_hangman():
    is_playing = True
    # All of the characters that the computer guessed wrong.
    guesses = ""

    # the number of guesses the computer has made.
    num_of_guesses = 0

    current_word = ""

    was_correct = True

    while is_playing:
        # Get input from the user if the current word on the board
        # changed or is new.
        if was_correct:
            print("What is currently on the board?")
            current_word = input("(Input unknown characters with _) ").lower()

        # if we found the word, we can stop playing.
        if current_word.count('_') is 0:
            break

        # Get all of the possible words that can be guessed
        possible_words = get_possible_words(guesses, current_word)

        print(f"There are {len(possible_words)} possible words.")

        # Print all of the possible words if there's not too many of them.
        if len(possible_words) <= 10:
            [print(word) for word in possible_words]

        # Early exit if it we only have one guess.
        if len(possible_words) is 1:
            print(f"It's obviously {possible_words[0]}.")
            break

        # Get the frequencies of each character in the possible words.
        stats = get_statistics(possible_words)

        # Remove characters we've already guessed from the statistics.
        [stats.pop(guessed_letter, None) for guessed_letter in guesses]

        print("Your most likely letter is...")

        likeliest_letter, likelihood = get_likeliest_letter(stats)

        print(f"{likeliest_letter} with a likelihood of {likelihood:.2f}%")

        was_correct = input("Was I correct? (y/n) ").lower() == 'y'

        # add our guess to the total listing of guesses.
        num_of_guesses += 1
        guesses += likeliest_letter

        # Print a new line to break each round up.
        print("")

    print(f"It took me {num_of_guesses} guesses to get it.")


if __name__ == '__main__':
    play_hangman()
\$\endgroup\$
1
  • \$\begingroup\$ Yup! There's definitely more comments than code :) \$\endgroup\$
    – Justin
    Jun 8, 2019 at 4:48

2 Answers 2

4
\$\begingroup\$

A starter note, throughout my answer: FP = "functional programming".

Possible Improvements

Use == rather than is for value comparisons.

I see if var is number in multiple places. Specifically, lines 29, 96, 109. One could argue that is reads better than ==, but the two are computationally different. (is breaks for large numbers.) Use == instead. See also: Is there a difference between “==” and “is”?

Thinking Functionally

FP tends to avoid mutability, i.e. changing state. FP is more about telling the computer what things are rather than what things do. Here are a couple lines of code from get_words (lines 9-12):

# filter the words so that they have the same number of characters as the word in play.
words = [word.lower() for word in words_temp if len(word) is word_len]
# Get rid of any possible duplicates in the file.
words = list(set(words))

Sweet, looks innocent. But you're assigning to words twice... you're changing its state.

I like how Miran Lipovača (author of a Haskell tutorial) puts it:

[Y]ou set variable a to 5 and then do some stuff and then set it to something else. [...] If you say that a is 5, you can't say it's something else later because you just said it was 5. What are you, some kind of liar?
(source)

We can actually trim your two lines down to one by directly using a set comprehension and thereby eliminating the mutation of words (note also the replacement of is with ==):

words = list({word.lower() for word in words_temp if len(word) == word_len})

You could even return the list directly from the function!

Next, another interesting snippet (lines 29-33):

if len(guesses) is 0:
    substitute = '.'
else:
    # exclude all of the wrong guesses
    substitute = f"[^{guesses}]"

This looks innocent too! But it also resembles an imperative statement: "if this, substitute is this, else substitute is that". We can make this more functional by clearly defining what substitute is:

substitute = '.' if len(guesses) == 0 else f"[^{guesses}]"

And this reads "substitute is '.' if this else that". (Note how the statement is now declarative and the variable becomes the subject of the statement.)

Yet another snippet (lines 113-117):

# Get the frequencies of each character in the possible words.
stats = get_statistics(possible_words)

# Remove characters we've already guessed from the statistics.
[stats.pop(guessed_letter, None) for guessed_letter in guesses]

Line 117 is a list comprehension, which is in itself functional... but it's changing the state of stats! Instead of removing the unneeded letters, make a new dictionary with the needed letters.

And back to my point: with functional programming, avoid mutability, define variables as what they are and not what they do/how they come about.

Game Loop and Mutability

The game loop... ah. It's a while-loop... and this presents a couple problems.

  1. While-loops tend to be imperative construct (telling the interpreter to loop while something isn't true).
  2. Since this is a dynamic game and since it's a single while-loop, you'll inevitably modify the state of surrounding variables to either keep track of progress.

    ## Line 81 ##
    # the number of guesses the computer has made.
    num_of_guesses = 0
    
    ## Line 127 ##
    # add our guess to the total listing of guesses.
    num_of_guesses += 1
    

    Lipovača: "But you just said num_of_guesses is 0!"

Solution? Recursion. Pass in the mutable variables as arguments to the function and recurse all the way to the end. (Or of course, you could stick with the more readable while-loop. Some things are inevitable – sigh.)

Consider using type-hints.

This is really helpful in the world of FP. What does a function receive? What does it return? This allows you to reason with the input and output of functions. See also: What are Type hints in Python 3.5

Comments

Yes, there are quite a lot. Some are unnecessary... and some of them are untruths.

Lines 20-21:

# Load the words in from the words.txt file.
words = get_words(num_of_characters)

What if the implementation of get_words changes? This comment becomes obsolete.

Lines 47-48:

# Print the list of possible words.
return possible_words

No printing here. You print it sometime later in the game loop, but not here. Here, there's only a return, which in itself doesn't do any printing.

Instead, consider commenting what each function does, preferably using Python doc-strings.

def get_words(word_len):
    """
    Returns a list of words each with length equal to `word_len`.
    """

As above, you can choose to leave out the details of the implementation. Sure, get_words will open, read, and close a file, but this won't have any side-effects1. Perhaps in the future, you might want to load the words from a database, and the doc-string won't need to be updated, because the input and output are unchanged.

1 – Unless if say, you're in a multi-threaded environment where the files will be accessed from different threads concurrently.

We also don't need the comment on line 20: # Load the words in from the words.txt file.. We can simply scroll to get_words and read the doc-string to know what it does.

See also: Self-Documenting Code; What is self-documenting code and can it replace well-documented code?

The Bright Side

Your program still has merits:

Use of Functions

Although all your functions are used only once, the functions clearly separate individual tasks, and this aids the reader to reason about the code.

Variables

Some are slightly redundant, but the names you've given them are helpful enough to remove at least a third of the comments.

Use of f-strings

f-strings are relatively new in Python, and they're not only more convenient, but also more functional over the OOP-variants: str.format and the %-printf notation.

Use of Comprehensions

I'm seeing quite a lot of comprehensions and no for-loop blocks. This is a merit: using a for-loop block with colon and suite bears the air of imperative programming (tells the interpreter to loop over an iterable), but comprehensions are more functional as they pack your values into a handy list/set/dictionary/generator expression.

PEP 8

Formatting is superb overall. What with snake_case, spacing, double line-breaks before and after functions, and a if __name__ == '__main__'. All this is good practice.

Keep it up!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you for your help! I'll take these suggestions and see if I can implement them. I was already thinking of using the typing module as I come from statically typed languages. Also thank you for the clarification between is and ==. I always get them confused and end up just using is. \$\endgroup\$
    – Duke0200
    Jun 8, 2019 at 16:31
1
\$\begingroup\$

apart from TrebledJ's excellent review of the functional improvements of your code, here some general Python improvements

get_words

There is no need for this function to return a list. The rets of your code only cares that it gets an iterable, so you might as well return the set. You can also avoid the lambda expression by doing map(str.strip, word_file) You can also incorporate the isalpha check here. Better to filter as soon as possible

This way this function can be reduced to:

def get_words(word_len):
    # Open the file with all the words in the English language.
    with open("words.txt") as word_file:
        # Get all the words without any newlines.
        return {
            word.lower()
            for word in map(str.strip, word_file)
            if len(word) == word_len and word.isalpha()
        }

get_statistics

Here you:

  • glue all the wordt together to one long string
  • get a sorted list of letters that you
  • get the unique letters
  • ask for each of these letters the count in the long string

This is very inefficient. There is no reason for the sorting, or the characters_in_words.

Also remember Python is batteries-included. itertools.chain and collections.Counter do all you need from this function

from itertools import chain
from collections import Counter
def get_statistics(possible_words) -> Counter:
    return Counter(chain.from_iterable(possible_words))

get_likeliest_letter

´collections.Counter´ also contains a most_common method that simplifies the ´get_likeliest_letter´

def get_likeliest_letter(stats: Counter):
    likeliest_letter, count = stats.most_common(1)[0]
    likelihood = count / sum(stats.values()) * 100.0
    return likeliest_letter, likelihood

get_possible_words

In each call to get_possible_words, you read all the words in words.txt. Since you can assume they don't change bewteen different tries, you can cache this, and pass it in the function as argument

There is also no need for the len(guesses) == 0. Just guesses as codition suffices. If it's an empty string, it is counted as False

Instead of the intermediary possible_matches, you can just return the list of words where current_word_regex.match(word) retuns a match object instead of None (which evaluates to False)

def get_possible_words(guesses, current_word, all_words):
    substitute = '.' if guesses else f"[^{guesses}]"
    # Make the current_word a regex phrase.
    current_word_regex = re.compile(current_word.replace('_', substitute))
    return [word for words in all_words if current_word_regex.match(word)]
\$\endgroup\$
1
  • \$\begingroup\$ Wow I honestly didn't realize I could shave down get_possible_words to just 3 lines (excluding comments). Also thank you for the tips on Counter. I tend to not use a ton of the Python library excluding what I've used above, so this will definitely help me with learning more of Python. Thank you for your help! \$\endgroup\$
    – Duke0200
    Jun 8, 2019 at 17:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.