patternpythonMinor
10 green bottles assignment
Viewed 0 times
greenassignmentbottles
Problem
This code was for a university assignment. It's well passed now so don't be scared of giving me false grades, for example. I was wondering what could be done better, or bits of code that are horribly inefficient or are bad practices.
```
def main():
import time
while True:
try:
num1 = int(input("Pick a number between 10 and 30: "))
except ValueError:
print("That's not a number!")
continue
if num1 > 30 or num1 < 10:
print("Not Valid!")
continue
break
while True:
try:
hue = str(input("Pick a colour; Red, Green, Blue: "))
except ValueError:
print("Letters not numbers!")
continue
if (hue == "Red") or (hue == "red") or (hue == "Green") or (hue == "green") or (hue == "Blue") or (hue == "blue"):
break
print("Generating File")
time.sleep(3)
numbers ='no', 'One', 'Two', 'Three', 'Four', 'Five', 'Six', 'Seven', 'Eight', 'Nine', 'Ten', 'Eleven', 'Twelve', 'Thirteen', 'Fourteen', 'Fifteen', 'Sixteen', 'Seventeen', 'Eighteen', 'Nineteen', 'Twenty', 'Twentyone', 'Twentytwo', 'Twentythree', 'Twentyfour', 'Twentyfive', 'Twentysix', 'Twentyseven', 'Twentyeight', 'Twentynine', 'Thirty'
text_one = hue +' bottle%s\nHanging on the wall'
text_two = "And if one " + hue + " bottle\nShould accidentally fall\nThere'll be"
text_three =' \n'
with open(numbers[num1] + ' ' + hue + ' Bottles.txt', 'w') as a:
for l in range(num1, 0, -1):
a.write(numbers[l]+ ' ')
if l == 1:
a.write(text_one % '' +'\n')
else:
a.write(text_one % 's' +'\n')
a.write(numbers[l] + ' ')
if l == 1:
a.write(text_one % '' + '\n')
else:
a.write(text_one % 's' + '\n')
a.write(text_t
```
def main():
import time
while True:
try:
num1 = int(input("Pick a number between 10 and 30: "))
except ValueError:
print("That's not a number!")
continue
if num1 > 30 or num1 < 10:
print("Not Valid!")
continue
break
while True:
try:
hue = str(input("Pick a colour; Red, Green, Blue: "))
except ValueError:
print("Letters not numbers!")
continue
if (hue == "Red") or (hue == "red") or (hue == "Green") or (hue == "green") or (hue == "Blue") or (hue == "blue"):
break
print("Generating File")
time.sleep(3)
numbers ='no', 'One', 'Two', 'Three', 'Four', 'Five', 'Six', 'Seven', 'Eight', 'Nine', 'Ten', 'Eleven', 'Twelve', 'Thirteen', 'Fourteen', 'Fifteen', 'Sixteen', 'Seventeen', 'Eighteen', 'Nineteen', 'Twenty', 'Twentyone', 'Twentytwo', 'Twentythree', 'Twentyfour', 'Twentyfive', 'Twentysix', 'Twentyseven', 'Twentyeight', 'Twentynine', 'Thirty'
text_one = hue +' bottle%s\nHanging on the wall'
text_two = "And if one " + hue + " bottle\nShould accidentally fall\nThere'll be"
text_three =' \n'
with open(numbers[num1] + ' ' + hue + ' Bottles.txt', 'w') as a:
for l in range(num1, 0, -1):
a.write(numbers[l]+ ' ')
if l == 1:
a.write(text_one % '' +'\n')
else:
a.write(text_one % 's' +'\n')
a.write(numbers[l] + ' ')
if l == 1:
a.write(text_one % '' + '\n')
else:
a.write(text_one % 's' + '\n')
a.write(text_t
Solution
A good place to start is to break your big main method into smaller chunks of work. Then you can focus on improving the code in each section.
Currently your main method does many things, it reads user input, builds up a song and writes it to a file. Here's how you might rewrite it:
The function is easier to read if all the calls it makes are at a similar level of abstraction. The main method doesn't do low level operations like input reading or file writing, it tells a story.
Currently your main method does many things, it reads user input, builds up a song and writes it to a file. Here's how you might rewrite it:
def main():
num_bottles = get_num_bottles_from_user()
hue = get_hue_from_user()
song = build_song(num_bottles, hue)
out_filename = create_out_filename(num_bottles, hue)
write_song_to_file(song, out_filename)The function is easier to read if all the calls it makes are at a similar level of abstraction. The main method doesn't do low level operations like input reading or file writing, it tells a story.
Code Snippets
def main():
num_bottles = get_num_bottles_from_user()
hue = get_hue_from_user()
song = build_song(num_bottles, hue)
out_filename = create_out_filename(num_bottles, hue)
write_song_to_file(song, out_filename)Context
StackExchange Code Review Q#93809, answer score: 8
Revisions (0)
No revisions yet.