patternpythonMinor
Madlibs Program
Viewed 0 times
madlibsprogramstackoverflow
Problem
I just started to learn Python, and wrote this Madlib program that works the way I intended it to, so I do not need help debugging, just some advice on tips to improve the code or make it simpler.
Program and Madlibs text files on Google Drive
Madlibs Reader
```
#Date 8SEP14
#introduction and file select
print ("Welcome to Madlibs!")
print ("Please select from the list below which Madlib Adventure you would like to do! ")
#display list of available madlibs
from os import listdir
for each in listdir():
if each.endswith('txt'):
print (each)
file_choice = input("What Madlib would you like to do? ")
#if input is not good then repeats until a good input is entered
while file_choice not in listdir():
file_choice = input("Please try again and don't forget the .txt at the end ")
#opening file and setting variables
with open(file_choice,"r",encoding = "utf-8") as my_file:
word_types = ["Verb","Adjective","Noun","Last-Name","Illness","Number","Place","Silly-Word","Body-Part","Past-Tense-Verb","Adverb","Verb-Ending-ING","Noun-Plural","Celebrity","Movie","Verb-Ending-ED","Mean-Nickname","Liquid","Cute-Animal-Plural","Article-of-Clothing","Celebrity-1","Celebrity-2"]
user_inputs = []
user_inputs_index = []
my_read_file = my_file.read()
new_read_file = my_read_file.split()
x = 0
for item in new_read_file:
if item in word_types and user_inputs_index == []:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item))
else:
if item in word_types:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item,(int(user_inputs_index[-1])+1)))
for each in user_inputs:
user_inputs[x] = input("Please Enter a(n) " + each + ": ")
x += 1
z = 0
for word in user_inputs:
y = int(user_inputs_index[z])
new_read_file[y] = word
z += 1
p
Program and Madlibs text files on Google Drive
Madlibs Reader
```
#Date 8SEP14
#introduction and file select
print ("Welcome to Madlibs!")
print ("Please select from the list below which Madlib Adventure you would like to do! ")
#display list of available madlibs
from os import listdir
for each in listdir():
if each.endswith('txt'):
print (each)
file_choice = input("What Madlib would you like to do? ")
#if input is not good then repeats until a good input is entered
while file_choice not in listdir():
file_choice = input("Please try again and don't forget the .txt at the end ")
#opening file and setting variables
with open(file_choice,"r",encoding = "utf-8") as my_file:
word_types = ["Verb","Adjective","Noun","Last-Name","Illness","Number","Place","Silly-Word","Body-Part","Past-Tense-Verb","Adverb","Verb-Ending-ING","Noun-Plural","Celebrity","Movie","Verb-Ending-ED","Mean-Nickname","Liquid","Cute-Animal-Plural","Article-of-Clothing","Celebrity-1","Celebrity-2"]
user_inputs = []
user_inputs_index = []
my_read_file = my_file.read()
new_read_file = my_read_file.split()
x = 0
for item in new_read_file:
if item in word_types and user_inputs_index == []:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item))
else:
if item in word_types:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item,(int(user_inputs_index[-1])+1)))
for each in user_inputs:
user_inputs[x] = input("Please Enter a(n) " + each + ": ")
x += 1
z = 0
for word in user_inputs:
y = int(user_inputs_index[z])
new_read_file[y] = word
z += 1
p
Solution
One thing I notice here is that you don't follow the DRY principle. Don't Repeat Yourself.
Always make an attempt to not repeat yourself because it a) cuts down on code, b) makes code easier to modify, and c) usually runs more efficiently. for example.
This function checks
Try changing it to:
PS instead of writing
Always make an attempt to not repeat yourself because it a) cuts down on code, b) makes code easier to modify, and c) usually runs more efficiently. for example.
for item in new_read_file:
if item in word_types and user_inputs_index == []:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item))
else:
if item in word_types:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item,(int(user_inputs_index[-1])+1)))This function checks
item in word_types two times in a row (which is inefficient), and in each case you do user_inputs.append(item) when you would do it regardless (which makes it harder to modify).Try changing it to:
for item in new_read_file:
if item in word_types:
user_inputs.append(item)
if user_inputs_index == []:
user_inputs_index.append(new_read_file.index(item))
else:
user_inputs_index.append(new_read_file.index(item,(int(user_inputs_index[-1])+1)))PS instead of writing
else: if...: you can simply write elif...:Code Snippets
for item in new_read_file:
if item in word_types and user_inputs_index == []:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item))
else:
if item in word_types:
user_inputs.append(item)
user_inputs_index.append(new_read_file.index(item,(int(user_inputs_index[-1])+1)))for item in new_read_file:
if item in word_types:
user_inputs.append(item)
if user_inputs_index == []:
user_inputs_index.append(new_read_file.index(item))
else:
user_inputs_index.append(new_read_file.index(item,(int(user_inputs_index[-1])+1)))Context
StackExchange Code Review Q#62356, answer score: 3
Revisions (0)
No revisions yet.