Code review comment for lp:~therp-nl/banking-addons/ba61-company_awareness_bank_import_settings
Revision history for this message
| Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
73416ba
(Get the code!)
> Hi Alexandre,
>
> thank you for the style notes. I followed them up only with a slight variation
> in the return statements of _default_company(). Additionally, I removed the
> void company_id keyword argument to this method.
Thanks a lot for taking my contribution into account.
Indeed, no one seems to pass that argument...
As for the return statement, there are way too many "X and Y or Z" in openerp related code. I understand the historical context, the need for a single statement in many places. IMO when a single statement is not mandatory (e.g. in a return statement) the full blown if: else: construct should be prefered. In other case, I would like to see a generalisation of the use of Python's ternary operator (Y if X else Z) which does not feature the pitfall of "X and Y or Z" (which returns Z if X is true and Y is false). I'm not asking for this in my reviews unless I spot a case where Y can be false. Do you have an opinion on this?