Reputation: 3031
This works, partially. More information may be needed, however, I thought I would post to get advice on anything obvious that might be wrong here.
The problem is that if activity.get_cost() returns a False
value, the function seems to exit entirely, returning None
.
What I'd like it to do, of course, is accumulate cost
Decimal values in the costs = []
and return their sum. Simple, I would have thought... but my novice Python skills are apparently missing something.
More information provided on request. Thank you.
def get_jobrecord_cost(self):
costs = []
for activity in self.activity_set.all():
cost = activity.get_cost()
if cost:
costs.append(cost)
if len(costs):
return sum(costs)
else:
return False
Upvotes: 1
Views: 362
Reputation: 198867
def get_jobrecord_cost(self):
return sum((activity.get_cost() or 0 for activity in activity_set.all()) or 0)
Depending on how much data you're dealing with, this version is just a bit more efficient than DNS's because it uses a generator comprehension and doesn't require loading up a whole list into memory. It's functionally equivalent to grieve's except the looping happens in C. Note that this doesn't necessarily mean this is better. This approach is obviously more dense and can be less readable.
Upvotes: 1
Reputation: 38219
I notice you're returning False if all the costs were None; I don't know if there's a specific reason for that, but it does make it a little bit harder to write. If that's not a requirement, you could write it like this:
def get_jobrecord_cost(self):
costs = [activity.get_cost() or 0 for activity in self.activity_set.all()]
return sum(costs)
Upvotes: 3
Reputation: 13518
I think you can simplify this with:
def get_jobrecord_cost(self):
costs = 0
for activity in self.activity_set.all():
cost = activity.get_cost()
if cost:
costs += cost
return costs
Upvotes: 2