3
\$\begingroup\$

I would like some feedback on my solution for the trip programming challenge. Please go through my code understanding that I am interested in interviewing for a dev position at some point and am also interested in competitive programming (I'm just getting started):

'''

@author: Teererai
This code is intended to calculate the minimum that a number of students needs to exchange
 in order to get the ammounts that they spend to within one cent of each other. The idea 
 is to calculate the average then round it up and down to the nearest cent. The next step 
 is to calculate the cumulative absolute positive and negative difference for each ammount spen
 from the nearest rounded average spend and take the maximum between the cumulative positive and negative difference.
'''
import sys
from sys import stdin
import math
from decimal import Decimal

def main():
    num_students=int(stdin.readline()) #read number of students 
    while num_students!=0: 
        i=num_students 
        total_spending=0
        student_spending=[] # stores amount spend by each student 
        pos_difference=0 # Keeps track of cumulative absolute difference between average spend and ammounts higher than the average.
        neg_difference=0 #Keeps track of cumulative negative difference. 
        for i in range(i):
            current_spending=float(stdin.readline())
            student_spending.append(current_spending)
            total_spending+=current_spending
        average_spending=total_spending/num_students 
        #these following two variables account for the fact that we can never get to the exact average.
        lower_average_spending=math.floor(average_spending*100)/100  
        upper_average_spending=math.ceil(average_spending*100)/100
        #As the challenge specifies, we are not aiming to get the ammount spend to the exact average but rather to either the nearest cent above and below it.  
        for element in student_spending:
            if element>average_spending:
                pos_difference=pos_difference+element-upper_average_spending
            else:
                neg_difference=neg_difference+lower_average_spending-element 
        amount_exchanged=Decimal(max(neg_difference,pos_difference))
        amount_exchanged=round(amount_exchanged,2)
        print("$",amount_exchanged,sep="")            
        num_students=int(stdin.readline())


    return 
main()
sys.exit(0)

p.s. the code works perfectly and has been accepted by the uva online judge. I am simply looking for advice.

\$\endgroup\$
  • 1
    \$\begingroup\$ Please edit your question to describe what it is that the program does. \$\endgroup\$ – ZeroOne May 8 '16 at 14:22
  • \$\begingroup\$ Comments added to code to explain what it does. Please also see the link to the programming challenge. \$\endgroup\$ – Teererai Marange May 8 '16 at 20:57
4
\$\begingroup\$

Readability counts

You follow PEP8 naming conventions but fail to follow layout ones. The result is a big block of code that get harder to read than necessary. Use spaces and blank lines to give the reader some rest.

You should also get yourself familiar with the if __name__ == '__main__' idiom and remove some unnecessary stuff:

  • return at the end of a function has no added value;
  • same for sys.exit(0) at the end of a program.

Break functionnality into tinier function

As it stand, your main function does 3 things:

  • Compute the required value;
  • Parse the input for information about computing such value;
  • Parse the input to repeat such computation for a number of trips.

You should use 3 functions instead, each responsible of its own task. Or at least split the "parse input for a trip and compute the value for this trip" part into its own function.

Define variables when you need them

Python is a dynamic language, you don't need to define variables before using them, nor preallocate memory for them. Just declare a new variable when you need it.

You should also aim at removing the number of intermediate variables that you use only once.

Use builtins

input, max, sum and list-comprehensions can help you express your thoughts in a more straighforward way.

str.format can help you with rounding and formatting output.

Proposed improvements

import math
from decimal import Decimal


def compute_expenses_for_a_trip(students_count):
    students_spending = [float(input()) for _ in range(students_count)]
    average_spending = sum(students_spending) / students_count

    # these following two variables account for the fact that we can never get to the exact average.
    lower_average_spending = math.floor(average_spending * 100) / 100  
    upper_average_spending = math.ceil(average_spending * 100) / 100

    pos_difference = sum(element - upper_average_spending for element in students_spending if element > average_spending)
    neg_difference = sum(lower_average_spending - element for element in students_spending if element < average_spending)
    return Decimal(max(pos_difference, neg_difference))


def trip_programming_challenge():
    while True:
        num_students = int(input())  # read number of students 
        if not num_students:
            break
        amount_exchanged = compute_expenses_for_a_trip(num_students)
        print('${:.2f}'.format(amount_exchanged))            


if __name__ == '__main__':
    trip_programming_challenge()

I didn't enforce a line length of 80 characters but you might want to.

I'm also unsure why you introduced Decimal that late into the computation. Either you use them all along or you remove them, but introducing them halfway through kind of defeat their purpose.

Also note that the two summed differences are mainly proposed as an alternative syntax but since they iterate two times over the same list you may find your for loop faster. If it matters, profile and choose the best.

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