4
\$\begingroup\$

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.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ (first time critiquing) i is a bad name. Spell out article. use a ternary operator too. Instead of text and new_text, I would have original_text and text and modify text. for both open calls put them in a context manager instead of calling .close(). You could speed up by calling instead reg_obj.find(text[last_found:]). Your code looks great without changes, but I think my suggestions would improve it. Keep up the good work. \$\endgroup\$ Sep 20 at 16:41
  • 4
    \$\begingroup\$ @RobertLugg that's quite valid as an answer \$\endgroup\$
    – Reinderien
    Sep 20 at 21:50
  • \$\begingroup\$ See also codereview.stackexchange.com/questions/208713/… \$\endgroup\$
    – Reinderien
    Sep 20 at 21:52

1 Answer 1

2
\$\begingroup\$

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)
\$\endgroup\$

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.