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

Reporting on Commissions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reportingcommissionsstackoverflow

Problem

I want to condense down some of this, is there anything I can change to make it better or shorter?

It works now but I don't know what else I can change, are any of my if statements changeable to be shorter?

def main():
    #displays company name and report
    print("Helena Hockey Haven")
    print("sales report")
    print()
    get_data()
    get_employees()
    get_sales()

#opens the file in read
def get_data():
    sales_numbers = open('SalesData.txt','r')
    num_list = sales_numbers.readlines()
    sales_numbers.close()
    index = 0
    while index  best_total:
                best_total = total
                employee=top
            total = 0
        if " " in line[index]  :
            split = name.split(', ')
            top=(split[1]+' '+split[0])
        if "." in line[index]  :
            total += float(line[index])
        index += 1
        #the employees name and sales numbers are printed 
    print(employee+" had the highest sales! $"+str(round(best_total,2)))
    print()
    print('|-------------------------------------------------------------------------------------------------|')
#the average sales are calculated and displayed
    index = 0
    total = 0
    average_sales = 0
    while index < len(line):
        if "." in line[index]  :
            total += float(line[index])
        index += 1
    average_sales+= total
    average_sales= average_sales/(round(index/13,0))
    print ("Average Sales: $"+str(round(average_sales,2)))
    print ('|-------------------------------------------------------------------------------------------------|')
main()

Solution

Code Review
Use the context manager to open files

And return list comprehensions, or better, generators - Instead of:

def get_data():
    sales_numbers = open('SalesData.txt','r')
    num_list = sales_numbers.readlines()
    sales_numbers.close()
    index = 0
    while index < len(num_list):
        num_list[index] = num_list[index].rstrip('\n')
        index += 1
    return num_list


do this (and use universal newlines mode with the U flag):

def get_data():    
    with open('SalesData.txt','rU') as sales_numbers:
        return [line.strip() for line in sales_numbers]


or perhaps a generator (doesn't materialize the whole thing in a list, but file stays open until it's exhausted):

def get_data():    
    with open('SalesData.txt','rU') as sales_numbers:
        for line in sales_numbers:
            yield line.strip()


Interesting issue raised by another answer:

They suggest map after calling readlines, which in Python 3 would return an iterator over a fully materialized dataset. The yield suggestion given above works fine in conjunction with using the file as an iterator (which makes sense, since you can think of a generator as a function that freezes at yield), but map does not - the file object gets closed when leaving the scope of the function:

>>> def fun2():
...     with open('foo', 'rU') as f:
...         return map(str.strip, f)
... 
>>> list(fun2())
Traceback (most recent call last):
  File "", line 1, in 
ValueError: I/O operation on closed file.


Misc

line = get_data() # I don't think this is semantically correct


You can set all of these to zero on the same line:

index = total = average = commission = bonus = 0


Python's for loops seem to escape you?

while index < len(line):
    name = line[index]


do this instead:

for name in get_data():


Use newlines:

These are redundant calls to the print function:

print("Helena Hockey Haven")
print("sales report")
print()


Instead, call it once, using newline characters:

print("Helena Hockey Haven\nsales report\n")


or maybe just use a multiline string:

print("""Helena Hockey Haven
sales report
""")


Conclusion

That's enough for now. Please ask if you have further questions.

Code Snippets

def get_data():
    sales_numbers = open('SalesData.txt','r')
    num_list = sales_numbers.readlines()
    sales_numbers.close()
    index = 0
    while index < len(num_list):
        num_list[index] = num_list[index].rstrip('\n')
        index += 1
    return num_list
def get_data():    
    with open('SalesData.txt','rU') as sales_numbers:
        return [line.strip() for line in sales_numbers]
def get_data():    
    with open('SalesData.txt','rU') as sales_numbers:
        for line in sales_numbers:
            yield line.strip()
>>> def fun2():
...     with open('foo', 'rU') as f:
...         return map(str.strip, f)
... 
>>> list(fun2())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: I/O operation on closed file.
line = get_data() # I don't think this is semantically correct

Context

StackExchange Code Review Q#113022, answer score: 8

Revisions (0)

No revisions yet.