views:

221

answers:

3

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
+2  A: 

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
grieve
+3  A: 

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)
DNS
+1  A: 
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.

Jason Baker