My code takes input from the user and creates a receipt with the information the program receives from the user:
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:
continue
Are there any ways to improve the efficiency of the code and shorten it? Is there anything I should improve in my code?
3 Answers 3
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 zip
built in but that is a more advanced topic.
pricing
I 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.
-
\$\begingroup\$ But the price would not be necessarily be an integer, it might be a float so I used an enter box instead \$\endgroup\$Anthony Pham– Anthony Pham2015年11月15日 21:02:20 +00:00Commented Nov 15, 2015 at 21:02
-
\$\begingroup\$ @PythonMaster indeed, answer edited to adress your remark \$\endgroup\$Caridorc– Caridorc2015年11月15日 21:03:35 +00:00Commented Nov 15, 2015 at 21:03
-
\$\begingroup\$ I mean not... Oops \$\endgroup\$Anthony Pham– Anthony Pham2015年11月15日 21:04:03 +00:00Commented Nov 15, 2015 at 21:04
-
1\$\begingroup\$ Ooh, dictation. Fun. Anyway, yeah, thanks for fixing it :D \$\endgroup\$anon– anon2015年11月15日 22:31:39 +00:00Commented Nov 15, 2015 at 22:31
Some tips for neater code:
if item == "done" or item == "None":
finished = True
can be rewritten as
finished = item in ["done", "None"]
this if (true): set x to true
pattern is ugly and overused in many languages.
if finished:
pass
else:
// compute stuff
Could be substituted by
if not finished:
// compute stuff
to improve code readability and make your intentions clearer.
Lastly, while 1
makes your intention slightly less clear than while True
.
In Python striving for English-like semantics whenever performance isn't critical is considered a common practice.
Is there anyways to improve the efficiency of the code and shorten it?
Before asking that ask if there is any need to improve the efficiency and shorten it. Readability nowadays adds much more value than performance tweaks. Writing code that your peers don't understand is a much bigger problem than doing inefficient loops.
-
5\$\begingroup\$ Or even finished = item in ["done", "None"] \$\endgroup\$The Cat– The Cat2015年11月16日 09:55:16 +00:00Commented Nov 16, 2015 at 9:55
-
\$\begingroup\$ @TheCat YES! And that also gets ride of that plethora of equal signs. Edited to include your suggestion. \$\endgroup\$Bernardo Sulzbach– Bernardo Sulzbach2015年11月16日 20:23:02 +00:00Commented Nov 16, 2015 at 20:23
Instead of maintaining multiple global arrays, they can be represented by a class and its members.
def class Item :
def __init__():
self.price = 0
self.name = ''
self.quantity=0