7
\$\begingroup\$

I am working on a Python project where a food processing company is trying to calculate its total sales for the year. Python has to read from a text file where its divided into for categories split by commas.

The first category is the Type of product, which can be cereal, chocolate candy etc produced by the company.

The second category is the brand of the said product, for example, Kaptain Krunch for cereal or Coco Jam for chocolate. The third category is the sales for the last fiscal year (2014) and the last category is sales for this fiscal year(2015). Note that only sales for fiscal year 2015 are to be calculated. The 2014 has no use in this program but it is there.

Here is how the text file looks like, called product.txt:

Cereal,Magic Balls,2200,2344

Cereal,Kaptain Krunch,3300,3123

Cereal,Coco Bongo,1800,2100

Cereal,Sugar Munch,4355,6500

Cereal,Oats n Barley,3299,5400

Sugar Candy,Pop Rocks,546,982

Sugar Candy,Lollipop,1233,1544

Sugar Candy,Gingerbud,2344,2211

Sugar Candy,Respur,1245,2211

Chocolate,Coco Jam,3322,4300

Chocolate,Larkspur,1600,2200

Chocolate,Mighty Milk,1234,2235

Chocolate,Almond Berry,998,1233

Condiments,Peanut Butter,3500,3902

Condiments,Hot Sauce,1234,1560

Condiments,Jelly,346,544

Condiments,Spread,2334,5644

What we are looking to do is to add the sales for Fiscal year 2015 by products and then the total sales for everything in 2015.

The output should look something like this in the written text file:

Total sales for cereal in 2015 : {Insert total number here}

Total sales for Sugar Candy in 2015 : {Insert total number here}

Total sales for Chocolate in 2015 : {Insert total number here}

Total sales for Condiments in 2015 : {Insert total number here}


Total sales for the company in 2015: {Insert total for all the products sold in 2015}

Along with that, it should also print the grand total on the Python run screen in the IDE along with the text file:

Total sales for the company in 2015: {Insert total for all the products sold in 2015}

Can I expand the program without using collections? Can I also expand the main function?

import collections

PRODUCT_FILE = "products.txt"
REPORT_FILE = "report.txt"

def main():
    # Easy way to get a dictionary where lookup defaults to 0
    categorycounts = collections.defaultdict(int)

    #open the files using with statements to ensure they're closed properly
    # without the need for an explicit call to close, even on exceptions
    with open(PRODUCT_FILE, newline='') as productfile,\
         open(REPORT_FILE, "w") as reportfile:
        rows = (line.rstrip().split(',')
        for line in productfile if line.rstrip())

        # Sum sales by product type letting csv parse
        # Filter removes empty rows for us; assume all other rows complete
        for category, brand, sales_lastyear, sales_thisyear in rows:
            categorycounts[category] += int(sales_thisyear)

        # Print categories in sorted order with their total sales
        for category, sales in sorted(categorycounts.items()):
            print('Total sales for', category, 'in 2015:', sales, file=reportfile)

        print('-'*80, file=reportfile) # Separator line between categories and total

        # Sum and print total sales to both file and screen
        totalsales = sum(categorycounts.values())
        print("Total sales for the company in 2015:", totalsales, file=reportfile)
        print("Total sales for the company in 2015:", totalsales)

if __name__ == '__main__':
    main()
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Here it is best to write a title that describes what the code accomplishes. I have made an edit to the title, feel welcome to review it. I hope you get some great answers! \$\endgroup\$ – Phrancis Feb 5 '16 at 21:14
  • 1
    \$\begingroup\$ Personally, I like this code, and think it's exactly how beginners should learn to program. \$\endgroup\$ – Barry Carter Feb 5 '16 at 22:56
  • \$\begingroup\$ I would like not to use collections as of yet and would like to expand the main method instead of using dunder(Double underscore). Also, with is a little bit confusing as well. Would you like to input in order to simplify this code? \$\endgroup\$ – user2954245 Feb 5 '16 at 22:59
2
\$\begingroup\$

Your code looks good, I have some minor suggestions:

Separate getting the data and outputting it

I would prefer reading something like this:

for product_info in product_informations(productfile):
    # Handle out-put of product_info

To separate concerns more clearly.

Use _ to signify un-used values

for category, brand, sales_lastyear, sales_thisyear in rows:
    categorycounts[category] += int(sales_thisyear)

The values brand and sales_lastyear are not used, so you do not need to give them a name:

for category, _, _, sales_thisyear in rows:
    categorycounts[category] += int(sales_thisyear)

Be explicit with file.write

If you want to write to a file, print will work just fine, but I suggest being the most explicit and using file.write.

This also helps separation of concerns because it forces formatting to be handled separately. (See str.format in the next paragraph)

Do not repeat yourself

    print("Total sales for the company in 2015:", totalsales, file=reportfile)
    print("Total sales for the company in 2015:", totalsales)

Only the output destination changes.

You can either save the message in a variable:

sales_info = "Total sales for the company in 2015:{}".format(totalsales)

reportfile.write(sales_info)
print(sales_info)

Or use a loop:

for destination in (sys.stdout, reportfile):
    print("Total sales for the company in 2015:{}".format(totalsales), file=destination)

.format is also preferred over str + str and print(x, y) for its extensibility and readibility.

Indent in generator expressions

It is weird to see a for at the start of a line as part of a generator comprehension, because it looks like a loop statement, some indenting will clarify that it is part of a generator comprehension:

rows = (line.rstrip().split(',')
            for line in productfile if line.rstrip())
\$\endgroup\$
  • \$\begingroup\$ Thank you. Is there a way to simplify main? \$\endgroup\$ – user2954245 Feb 8 '16 at 17:07
1
\$\begingroup\$

Overall, I would say this is excellent quality code, and (as @Barry Carter commented) is exactly how people should learn Python.

I agree with the issues highlighted by @Caridorc, particularly the indent for the for loop generator (this line threw me until I looked twice at the braces).


As you said in your comment, you probably don't want to use collections here (just for default values). Luckily there is a built-in get method of dict that you can use instead!

The Python dictionary structure (dict) has, since PEP 464 (circa ~2014), had a 2-argument form of the get method. The second argument is a default value (which is returned instead of raising KeyError on lookup's of non-existant keys.

You can see an example of it here. But basically, you can replace the line:

categorycounts[category] += int(sales_thisyear)

with

categorycounts[category] = categorycounts.get(category,0) +int(sales_thisyear)

(Granted, it is a little more verbose without the += syntax, but does allow you to skip using collections (and therefore skip explaining the import statement ;)

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