HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python code for Mad Libs

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
madforpythoncodelibs

Problem

I am a beginner with only about 2 months under my belt. I'm learning Python 3 using the Automate the Boring Stuff with Python manual. In the chapter on reading and writing files one of the exercises is to make a program that would fill in a mad lib that was read from a file. It works, but it's not very elegant. Can anyone offer me advice on how to better do this task or a better direction to go?

# madLibs.py

import re, os

# define regex
verbRegex = re.compile(r'verb', re.I)
nounRegex = re.compile(r'noun', re.I)
adjectiveRegex = re.compile(r'adjective', re.I)

# change directory 
os.chdir('C:\\MyPythonScripts')

# read contents of madLib.txt and store it in variable madLib
madLibFileObject = open('madLib.txt')
madLib = madLibFileObject.read()

y = 0
newMadLib = []

for word in madLib.split():
    newMadLib.append(word)
    if verbRegex.search(word) != None:
        x = input('Enter VERB:  ')
        newMadLib[y] = x
    elif nounRegex.search(word) != None:
        x = input('Enter NOUN:  ')
        newMadLib[y] = x
    elif adjectiveRegex.search(word) != None:
        x = input('Enter ADJECTIVE:  ')
        newMadLib[y] = x
    y += 1

newMadLib = (' ').join(newMadLib)

print(madLib)
print (newMadLib)

Solution

It is unnecessary to change the current directory in your python script. Instead you can just open the file with an absolute path - i.e.

madLibFileObject = open("C:\\MyPythonScripts\\madLib.txt")`


To avoid leaking files (madLibFileObject), it it a good idea to use a with statement whenever you open a file, which takes care of automatically closing it for you:

with open("C:\\MyPythonScripts\\madLib.txt") as madLibFileObject:
     madLib = madLibFileObject.read()


Using three regular expressions that each match one word is overkill for this task. It is instead better to use word.lower() == "verb"

for word in madLib.split():
    newMadLib.append(word)
    if word.lower() == "verb":
        x = input('Enter VERB:  ')
        newMadLib[y] = x
    elif word.lower() == "noun":
        x = input('Enter NOUN:  ')
        newMadLib[y] = x
    elif word.lower() == "adjective":
        x = input('Enter ADJECTIVE:  ')
        newMadLib[y] = x


Then, there is clearly quite a bit of code duplication in the above loop, so it should be shortened using a list:

PARTS_OF_SPEECH = ["verb", "adjective", "noun"]
for word in madLib.split():
    if word.lower() in PARTS_OF_SPEECH:
        newMadLib[y] = input("Enter " + word.upper())


Your y counter variable is a reimplementation of the enumerate built-in function, in addition to being poorly named. index would be a better name:

for index, word in enumerate(madLib.split()):


Full code:

PARTS_OF_SPEECH = ["verb", "noun", "adjective"]
with open("C:\\MyPythonScripts\\madLib.txt") as madLibFileObject:
     madLib = madLibFileObject.read()
newMadLib = []
for index, word in enumerate(madLib.split()):
     if word.lower() in PARTS_OF_SPEECH:
        newMadLib[index] = input("Enter " + word.upper())
newMadLib = ' '.join(newMadLib)
print(madLib)
print(newMadLib)

Code Snippets

madLibFileObject = open("C:\\MyPythonScripts\\madLib.txt")`
with open("C:\\MyPythonScripts\\madLib.txt") as madLibFileObject:
     madLib = madLibFileObject.read()
for word in madLib.split():
    newMadLib.append(word)
    if word.lower() == "verb":
        x = input('Enter VERB:  ')
        newMadLib[y] = x
    elif word.lower() == "noun":
        x = input('Enter NOUN:  ')
        newMadLib[y] = x
    elif word.lower() == "adjective":
        x = input('Enter ADJECTIVE:  ')
        newMadLib[y] = x
PARTS_OF_SPEECH = ["verb", "adjective", "noun"]
for word in madLib.split():
    if word.lower() in PARTS_OF_SPEECH:
        newMadLib[y] = input("Enter " + word.upper())
for index, word in enumerate(madLib.split()):

Context

StackExchange Code Review Q#129893, answer score: 4

Revisions (0)

No revisions yet.