Review: Needs Information code review, no test

I notice no context is provided and therefore none is propagated. 


Regarding the test 'if (not country_code) or not country_id' : 

* would "if not (country_code and country_id)" be more readable? (not sure 
about this, and see 2nd point below)

* from the code higher in the function, we get there if a country_code was 
provided but the corresponding country was not found. My understanding of the 
execution flow leads me to think  that the first part of the test is to guard 
against a NameError because country_id is only defined if country_code was 
defined. In that case, I recommend setting country_id to False at the beginning 
of the function, which allows to rewrite the test as "if not country_id"



-- 
https://code.launchpad.net/~therp-nl/banking-addons/ba61-multicompany_safe_partner_search/+merge/146795
Your team Banking Addons Team is subscribed to branch lp:banking-addons.

-- 
Mailing list: https://launchpad.net/~banking-addons-team
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~banking-addons-team
More help   : https://help.launchpad.net/ListHelp

Reply via email to