Code review comment for lp:~camptocamp/account-invoicing/account-invoicing_payment_term_rounding

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

s/rouding/rounding/ in __openerp__.py

In this code:

    def compute_line_amount(self, cr, uid, id, total_amount, remaining_amount, context=None):
        if isinstance(id, list):
            id = id[0]

I observed that tuple is usually an accepted type for the ids too.
When 'id' is a list, you should assert that the received length if 'id' is 1. Otherwise the programmer using your method might have unpredictable effects if he want to call it on multiple ids.

Proposal:
    def compute_line_amount(self, cr, uid, id, total_amount, remaining_amount, context=None):
        if isinstance(id, (tuple, list)):
            assert len(id) == 1, "compute_line_amount accepts only 1 ID"
            id = id[0]

When you format the dates, '%Y-%m-%d' should be replaced by openerp.tools.DEFAULT_SERVER_DATE_FORMAT

At l.157, the whole branch
    if amt:
        ....
Can be replace by
    if not amt:
        continue
    ...
This will avoid to nest all this part of code and you'll gain some width.

review: Needs Fixing

« Back to merge proposal