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

asked May 8, 2016 at 11:22
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please edit your question to describe what it is that the program does. \$\endgroup\$ Commented May 8, 2016 at 14:22
  • \$\begingroup\$ Comments added to code to explain what it does. Please also see the link to the programming challenge. \$\endgroup\$ Commented May 8, 2016 at 20:57

1 Answer 1

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

answered May 9, 2016 at 15:23
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.