patternpythonMinor
Python Receipt Creator
Viewed 0 times
creatorpythonreceipt
Problem
My code takes input from the user and creates a receipt with the information the program receives from the user:
Are there any ways to improve the efficiency of the code and shorten it? Is there anything I should improve in my code?
import easygui
items = []
prices = []
amount = []
amounts = 0
finished = False
total_price = 0.00
list_num = -1
def take_item():
global list_num, amounts, finished
item = easygui.enterbox("What item is it?")
if item == "done" or item == "None":
finished = True
else:
how_many = easygui.integerbox("How many {0:} do you have?".format(item))
amounts = how_many
items.append(item)
amount.append(how_many)
list_num += 1
def pricing():
global list_num, total_price
if finished:
pass
else:
try:
price = easygui.enterbox("What is the price of 1 {0:}".format(items[list_num]))
new_price = float(price)
except ValueError:
easygui.msgbox("Please enter a number.")
pricing()
else:
total_price += float(price) * amounts
prices.append(new_price)
def create_receipt():
print "Item \t Amount \t Price \t Total Price"
for i in range(0, list_num+1):
print items[i], "\t\t", amount[i], "\t\t 3", prices[i]
print "\t\t\t ", "$",total_price
print "Thank you for using Tony's Receipt Maker!"
print "-----------------------------------------------------------"
while 1:
take_item()
pricing()
if finished:
create_receipt()
else:
continueAre there any ways to improve the efficiency of the code and shorten it? Is there anything I should improve in my code?
Solution
Globals
The problem is with your global variables.
You have way too many global variables and this is a problem because a global variable may, by definition, be modified from anywhere in your program.
This means that the reader of your program has to keep in mind all the values of all the variables at the same time.
Now for a small script like this this is no big deal, but imagine having not 5 global variables but 50 of them. Now you understand how hard it can become to follow un-organized code.
I suggest that each function should take the variables that it operates on as arguments and, if it modifies them, then it should return the new values back.
For example the function that prints out the final recipe can easily be adjusted to take as arguments the three arrays.
Premature optimization
I see that you keep a variable that indicates how long the input list is; that is premature optimization because you may just recalculate the length just before looping.
In fact in a user interface driven program you should not worry at all about performance because computers are very very fast nowadays.
To be complete there is an even more efficient way of looping, that is using the
I also had a look at your
I think the whole
This will allow you to reuse that particular piece of code.
The problem is with your global variables.
You have way too many global variables and this is a problem because a global variable may, by definition, be modified from anywhere in your program.
This means that the reader of your program has to keep in mind all the values of all the variables at the same time.
Now for a small script like this this is no big deal, but imagine having not 5 global variables but 50 of them. Now you understand how hard it can become to follow un-organized code.
I suggest that each function should take the variables that it operates on as arguments and, if it modifies them, then it should return the new values back.
For example the function that prints out the final recipe can easily be adjusted to take as arguments the three arrays.
Premature optimization
I see that you keep a variable that indicates how long the input list is; that is premature optimization because you may just recalculate the length just before looping.
In fact in a user interface driven program you should not worry at all about performance because computers are very very fast nowadays.
To be complete there is an even more efficient way of looping, that is using the
zip built in but that is a more advanced topic.pricingI also had a look at your
pricing function.I think the whole
try except block should be extracted into its own function as getting an input that must be a number from the user is a pretty common task.This will allow you to reuse that particular piece of code.
Context
StackExchange Code Review Q#110850, answer score: 6
Revisions (0)
No revisions yet.