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.
-
1\$\begingroup\$ Please edit your question to describe what it is that the program does. \$\endgroup\$ZeroOne– ZeroOne2016年05月08日 14:22:54 +00:00Commented 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\$Teererai Marange– Teererai Marange2016年05月08日 20:57:06 +00:00Commented May 8, 2016 at 20:57
1 Answer 1
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.