← Back to team overview

banking-addons-team team mailing list archive

Re: lp:~therp-nl/banking-addons/ba61-company_awareness_bank_import_settings into lp:banking-addons

 

> 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? 
-- 
https://code.launchpad.net/~therp-nl/banking-addons/ba61-company_awareness_bank_import_settings/+merge/145023
Your team Banking Addons Team is subscribed to branch lp:banking-addons.


References