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?

\$\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.

\$\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\$ Nov 15, 2015 at 21:02
  • \$\begingroup\$ @PythonMaster indeed, answer edited to adress your remark \$\endgroup\$
    – Caridorc
    Nov 15, 2015 at 21:03
  • \$\begingroup\$ I mean not... Oops \$\endgroup\$ Nov 15, 2015 at 21:04
  • 1
    \$\begingroup\$ Ooh, dictation. Fun. Anyway, yeah, thanks for fixing it :D \$\endgroup\$
    – anon
    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.

\$\endgroup\$
2
  • 5
    \$\begingroup\$ Or even finished = item in ["done", "None"] \$\endgroup\$
    – The Cat
    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\$ 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
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.