Automate the Boring Stuff Chapter 9 - Mad Libs Exercise

Question

The project outline:

Create a Mad Libs program that reads in text files and lets the user add their own text anywhere the word ADJECTIVE, NOUN, ADVERB, or VERB appears in the text file. For example, a text file may look like this:

The ADJECTIVE panda walked to the NOUN and then VERB. A nearby NOUN was unaffected by these events.

The program would find these occurrences and prompt the user to replace them.

Enter an adjective: silly Enter a noun: chandelier Enter a verb: screamed Enter a noun: pickup truck

The following text file would then be created:

The silly panda walked to the chandelier and then screamed. A nearby pickup truck was unaffected by these events.

The results should be printed to the screen and saved to a new text file.

My solution:

import pyinputplus as pyip
import re

text_file = open("mad_libs_text.txt", "r")
text = text_file.read()
reg_obj = re.compile(r"ADJECTIVE|NOUN|VERB")
mo = reg_obj.findall(text)
new_text = text

for i in mo:
    art = "a"
    if i == "ADJECTIVE":
        art = "an"
    user_inp = pyip.inputStr(f"Enter {art} {i.lower()}\n")
    new_text = re.sub(i, user_inp, new_text, count=1)

print(new_text)
new_text_file = open("mad_libs_new.txt", "w")
new_text_file.write(new_text)
text_file.close()
new_text_file.close()

Thanks in advance for the critique.

Answer 1

Use descriptive names

It's good to use descriptive names for variables (and everything) to help readers understand the elements of the code and how they might interact. Names like reg_obj, mo, i, art don't describe what's stored in them, so readers have to understand how they are used to deduce their purpose.

With descriptive names, the code is a lot easier to understand:

re_word_types = re.compile(r"ADJECTIVE|NOUN|VERB")
word_types = re_word_types.findall(text)
new_text = text

for word_type in word_types:
    article = "a"
    if word_type == "ADJECTIVE":
        article = "an"
    word = pyip.inputStr(f"Enter {article} {word_type.lower()}\n")
    new_text = re.sub(word_type, word, new_text, count=1)

Use more strict patterns

I suspect that you want to replace the word types when they are not part of other words. For example you probably don't want to replace the NOUN in PRONOUNCE, or VERB in VERBAL. Such unintended replacements can be prevented using stricter patterns, for example:

re_word_types = re.compile(r"\b(ADJECTIVE|NOUN|VERB)\b")

By enclosing the terms within \b...\b, we ensure that they will only be matched at proper word boundaries.

Use context manager for file operations

Instead of:

text_file = open("mad_libs_text.txt", "r")
text = text_file.read()
text_file.close()

The recommended idiom:

with open("mad_libs_text.txt", "r") as text_file:
    text = text_file.read()

And then no need to call .close() explicitly, the context manager will do it automatically when leaving the scope of the with.

Replace words all at once

The posted code repeatedly replaces one term at a time in the text. With 4 terms in the text, the text is re-processed 4 times.

Another alternative is to replace all the patterns in a single pass, by using another form of re.sub, with a callable. However, to be able to replace all the terms at once, the words needed in the replacement must all be collected up front. So this is a tradeoff of the two techniques.

def replacer(match):
    word_type = match.group()
    word = words[word_type].popleft()
    return word


words = defaultdict(deque)

for word_type in word_types:
    article = "a"
    if word_type == "ADJECTIVE":
        article = "an"
    word = pyip.inputStr(f"Enter {article} {word_type.lower()}\n")
    words[word_type].append(word)

new_text = re_word_types.sub(replacer, text)