HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python Receipt Creator

Submitted by: @import:stackexchange-codereview··
0
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:

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?

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

Context

StackExchange Code Review Q#110850, answer score: 6

Revisions (0)

No revisions yet.