A Simple Rewrite (How I Code)

Note: I just found this unpublished post from a couple of years ago. I think I wanted to polish it a bit before publishing, but it looks fine to me now.

I’m taking another shot at the “How I Code” series of posts. This isn’t a preachy “How everyone should code” post. Instead, this is a snapshot of how I look at code now, at this point in time. Next week I could be doing things differently.

Here’s an example from Satchmo, the web shopping cart software that is built on the Django framework. A bug involving MySQL was recently patched in the Satchmo code base. The bug fix itself isn’t very interesting, but the remaining code is. Here’s the post-fix code:

def payments_completed(self):
    q = self.payments.exclude(transaction_id__isnull = False, transaction_id = "PENDING")
    result = [p for p in q if p.amount]
    return result

Naturally, I have questions.

Why does the ‘result’ variable exist?

def payments_completed(self):
    q = self.payments.exclude(transaction_id__isnull = False, transaction_id = "PENDING")
    return [p for p in q if p.amount]

What are ‘p’ and ‘q’?

def payments_completed(self):
    query_set = self.payments.exclude(transaction_id__isnull = False, transaction_id = "PENDING")
    return [payment for payment in query_set if payment.amount]

Would a non-programmer understand this?

def payments_completed(self):
    payments = self.payments.exclude(transaction_id__isnull = False, transaction_id = "PENDING")
    return [payment for payment in payments if payment.amount]

Don’t we already have a ‘payments’ variable?

def payments_completed(self):
	return self.payments.exclude(transaction_id__isnull = False, transaction_id = "PENDING", amount__isnull = false)

I think that’s better. What do you think?