7
\$\begingroup\$

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?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Nov 15, 2015 at 20:23
\$\endgroup\$

3 Answers 3

6
\$\begingroup\$

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.

answered Nov 15, 2015 at 20:50
\$\endgroup\$
4
  • \$\begingroup\$ But the price would not be necessarily be an integer, it might be a float so I used an enter box instead \$\endgroup\$ Commented Nov 15, 2015 at 21:02
  • \$\begingroup\$ @PythonMaster indeed, answer edited to adress your remark \$\endgroup\$ Commented Nov 15, 2015 at 21:03
  • \$\begingroup\$ I mean not... Oops \$\endgroup\$ Commented Nov 15, 2015 at 21:04
  • 1
    \$\begingroup\$ Ooh, dictation. Fun. Anyway, yeah, thanks for fixing it :D \$\endgroup\$ Commented Nov 15, 2015 at 22:31
4
\$\begingroup\$

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.

answered Nov 15, 2015 at 23:10
\$\endgroup\$
2
  • 5
    \$\begingroup\$ Or even finished = item in ["done", "None"] \$\endgroup\$ Commented 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\$ Commented Nov 16, 2015 at 20:23
1
\$\begingroup\$

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
answered Nov 16, 2015 at 2:06
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.