Skip to main content
Code Review

Return to Answer

used a softer tone, elaborated, consistency
Source Link
TrebledJ
  • 437
  • 2
  • 11

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. (Actually, is breaks for large numbers.) Use == instead. See also: Is there a difference between "==" and "is"?

Functional Programming and ImmutabilityThinking Functionally

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.)

  1. While-loops tend to be imperative construct (telltelling 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, whether the game is finished or not, etc.
    ## 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.)

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

Yes, there are too manyquite a lot. AndSome are unnecessary... and some of them are outright (削除) lies (削除ここまで) redundantuntruths.

Lines 20-21: What if the implementation of get_words changes? This comment becomes obsolete.

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

Lines 47-48: No printing here.

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, commentconsider 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`.
 """

We don't needAs above, you can choose to bother withleave out the details of the implementation since opening. Sure, readingget_words will open, read, and closingclose a file, but this won't have any side effectsside-effects1. Perhaps in the future, you might want to load the words from a database, and the docstringdoc-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 docstringdoc-string to know what it does.

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

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

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

Functional Programming and Immutability

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

  1. While-loops tend to be imperative construct (tell 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, whether the game is finished or not, etc.
    ## 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.

This is really helpful in the world of FP. What does a function receive? What does it return? See also: What are Type hints in Python 3.5

Yes, there are too many. And some of them are outright (削除) lies (削除ここまで) redundant.

Lines 20-21: What if the implementation of get_words changes? This comment becomes obsolete.

Lines 47-48: No printing here.

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

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

We don't need to bother with the implementation since opening, reading, and closing a file won't have any side effects. Perhaps in the future, you might want to load the words from a database, and the docstring won't need to be updated.

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 docstring to know what it does.

I'm seeing quite a lot of comprehensions and few for-loop blocks. This is good. Using a for-loop block with colon and suite bears the air of imperative programming (tell the interpreter to loop over this), but comprehensions are more functional.

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

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.)

  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.)

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

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

Lines 20-21:

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

Lines 47-48:

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?

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.

added 13 characters in body
Source Link
TrebledJ
  • 437
  • 2
  • 11

Keep it up!

Keep it up!

Source Link
TrebledJ
  • 437
  • 2
  • 11

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, but the two are computationally different. (Actually, is breaks for large numbers.) Use == instead. See also: Is there a difference between "==" and "is"?

Functional Programming and Immutability

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 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 (tell 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, whether the game is finished or not, etc.
    ## 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.

Consider using type-hints.

This is really helpful in the world of FP. What does a function receive? What does it return? See also: What are Type hints in Python 3.5

Comments

Yes, there are too many. And some of them are outright (削除) lies (削除ここまで) redundant.

Lines 20-21: What if the implementation of get_words changes? This comment becomes obsolete.

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

Lines 47-48: No printing here.

# Print the list of possible words.
return possible_words

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

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

We don't need to bother with the implementation since opening, reading, and closing a file won't have any side effects. Perhaps in the future, you might want to load the words from a database, and the docstring won't need to be updated.

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 docstring to know what it does.

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 few for-loop blocks. This is good. Using a for-loop block with colon and suite bears the air of imperative programming (tell the interpreter to loop over this), but comprehensions are more functional.

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.

lang-py

AltStyle によって変換されたページ (->オリジナル) /