8
\$\begingroup\$

This is my invoicing program I made for myself to keep track of purchases people made from me. (Yes I know eBay does this for me but I wanted to see what I could make.) Basically it can search the invoice file for a keyword (the invoice number) or lets you enter a new invoice to a file named usb.txt. It is usb.txt because I was selling USB cables at the time.

#Invoice Entering Program Ebay

print("Ebay Invoice Program\n")


while True:
    file = input("Enter 0 to Quit. Enter S to Search The File. Press ENTER to Input new Entries *Case Sensitive*: " )
    if file == "0":
        print("Exiting Program...")
        break
    elif file=="S":
        keyword = input("Enter Invoice Number to Search For: ")
        look_in = "usb.txt"
        search_file = open(look_in,"r")
        with open(look_in, "r") as search_file:
            for line in search_file:
                if keyword in line:
                    print(line)

    else:

        Name = input("Enter Buyer Name: ")
        InNum = input("Enter Ebay Invoice Number: ")
        Quant = int(input("Enter Amount Bought: "))
        Price = Quant * 5
        with open("usb.txt", "at")as out_file:
            out_file.write(
                "Name: %s || Invoice Number: %s || Quantity: %s || Price:$ %s \n\n" % (Name, InNum, Quant, Price))
            out_file.close()
            print("File edits made")

I'm pretty new to python and programming in general. If you have any suggestions on how I could improve it or any ideas to add functionality I'd love to hear it! This was written in python 3.5.1.

\$\endgroup\$
8
\$\begingroup\$

What happens if the user types "s"? Your program says:

Enter Buyer Name: 

Oops! It was supposed to be uppercase, wasn't it? I'm not sure why you want it to be case-sensitive, but you should tell the user of his mistake instead of just going on as if you know what he meant. That means, I would change else: to elif not file: and add an else: that prints something like Sorry, I don't know what that means.


Your search_file = ... line is useless because you redefine search_file on the next line (with open(...) as search_file:). By the way, I am happy to see that you are using with blocks. That is rare, but good practice.


Your naming is not consistent with PEP 8, the Python style guide, on Method Names and Instance Variables:

Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability...


You use int(input(...)) without a try block. That is dangerous. If the user types something that is not a valid integer, there will be a very nasty error. You should use a while loop that keeps going until the user types a valid integer. The validity test would, of course, be with a try-except block.


out_file.close() is unnecessary in your with block. The with block will take care of closing the file.


You might want to take a look at the csv module. With its help, you are more flexible with the format in which the entries are printed. Your printing code might end up looking something like this:

template = "Name: {name} || Invoice Number: {invoice} || Quantity: {quantity} || Price: ${price}\n\n"

...

with open(look_in, "rt") as search_file:
    for line in csv.DictReader(search_file):
        if line['invoice'] == keyword:
            print(template.format(**line))

Your writing code:

with open("usb.txt", "at") as out_file:
    writer = csv.DictWriter(out_file, (
      'name', 'invoice', 'quantity', 'price')):

    writer.writeheader()
    writer.writerow({
        'name': Name, 'invoice': InNum, 'quantity': Quant,
        'price': Price})

As it is, an entry might be displayed just because its name includes the invoice number. With a csv file, that is no longer possible.

\$\endgroup\$
2
\$\begingroup\$

Constants on top

look_in = "usb.txt"
...
Price = Quant * 5

The file to look in may reasonably change in the future as may the price per unit (currently 5).

I suggest writing the constants on top in UPPERCASE to allow easier modification of the program.

Useless .close()

with closes the file automatically so you can remove the close at the end of your code.

Functionality modularization

Finding the lines where a string is present given a text is one unit of work that is independent from the rest of the logic, so I would write a function for it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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