1
\$\begingroup\$

The following method is designed to display what we owe to a vendor based on 5 buckets. Basically, an aged trial balance (totals only). It also needs to be able to display the data based on any date. It shows me what the trial balance would have looked like on April 1st. By default it displays based on the date that it is run. The if and elif branching "smells".

def _get_balances(self,ref_date=None):
        if not ref_date:
            ref_date = datetime.date.today()
        totals = [Decimal('0.0'),Decimal('0.0'),Decimal('0.0'),Decimal('0.0'),Decimal('0.0')]
        for invoice in self.invoices:
            if invoice.date:
                payments = Decimal('0.0')
                for payment in invoice.payments:
                    if payment.check.date <= ref_date:
                        payments += payment.amount
                invoice_total = invoice.amount - payments
                if invoice.date >= ref_date - relativedelta(days=29):
                    totals[0] += invoice_total
                elif invoice.date <= ref_date - relativedelta(days=30) and \
                    invoice.date >= ref_date - relativedelta(days=43):
                        totals[1] += invoice_total
                elif invoice.date <= ref_date - relativedelta(days=44) and \
                    invoice.date >= ref_date - relativedelta(days=47):
                        totals[2] += invoice_total
                elif invoice.date <= ref_date - relativedelta(days=48) and \
                    invoice.date >= ref_date - relativedelta(days=59):
                        totals[3] += invoice_total
                elif invoice.date <= ref_date - relativedelta(days=60):
                    totals[4] += invoice_total
        return tuple(totals)
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

I wrote methods for determining if the date falls within the desired range. This doesn't really solve the cluster of if else statements, but it is more readable.

I also used a few built-in Python functions, filter and reduce. These are tailor-made for handling these kinds of boiler-plate tasks.

I also changed if ref_date to if ref_date is None. Quoted from PEP 8

beware of writing if x when you really mean if x is not None

Finally, I replaced your list initialization for totals to according to this SO question.

def _get_balances(self,ref_date=None):
    if ref_date is None:
        ref_date = datetime.date.today()
    totals = [Decimal('0.0')] * 5
    for invoice in self.invoices:
        if invoice.date is not None:
            payments = filter(lambda: payment.check.date <= ref_date)
            total_amount_paid = reduce(lambda: total, payment: payment.amount + total, payments, 0)
            invoice_total = invoice.amount - total_amount_paid 
            if self.__invoice_date_is_before(invoice.date, ref, date, 29):
                totals[0] += invoice_total
            elif self.__invoice_date_is_between(invoice.date, ref_date, 30, 43):
                totals[1] += invoice_total
            elif iself.__invoice_date_is_between(invoice.date, ref_date, 44, 47):
                totals[2] += invoice_total
            elif self.__invoice_date_is_between(invoice.date, ref_date, 48, 59):
                totals[3] += invoice_total
            elif self.__invoice_date_is_after(invoice.date, ref_date, 60):
                totals[4] += invoice_total
    return tuple(totals)

def __invoice_date_is_between(self, invoice_date, ref_date, start_of_period, end_of_period):
    return invoice_date <= ref_date - relativedelta(days=start_of_period) and \
                invoice_date >= ref_date - relativedelta(days=end_of_period)

def __invoice_date_is_after(self, invoice_date, ref_date, end_of_period):
    return invoice_date <= ref_date - relativedelta(days=end_of_period):

def __invoice_date_is_before(self, invoice_date, ref_date, start_of_period):
    return invoice_date >= ref_date - relativedelta(days=start_of_period)
\$\endgroup\$
2
\$\begingroup\$

Firstly, everything John Thomas Evans said.

However,

            payments = Decimal('0.0')
            for payment in invoice.payments:
                if payment.check.date <= ref_date:
                    payments += payment.amount

I'd write it as:

            payments = [payment.amount for payment in invoice.payments if payment.check.date <= ref_date]
            payments = sum(payments, Decimal("0"))

And for your big mess of ifs...

days_before_reference_date = (ref_date - invoice.date).days
sections = [30, 44, 48, 60]
section = bisect.bisect_right(sections, days_before_reference_date)
totals[section] += invoice_total

Basically, sections contains the starting points for the different buckets. We use bisect to detect which bucket our given value would fall into.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.