"if not <variable>" instead of "if <variable> is None"

[Leonardo Santagada @ Proge.com.br]

In a lot of places (I greped "if +not +\w+:" and found tons of these) the function definition says <variable>=None and then check if the variable is false then creates an empty var. In python empty vars are usually false, so if someones passes an empty var the function will create a new one, both creating more objects than it needs to and making it way harder to test openerp code. Changing it to if <variable> is None is both more semantically correct and will make the problem go away.

[xrg (xrg)]

On Friday 22 July 2011, you wrote:
> Public bug reported:
>
> In a lot of places (I greped "if +not +\w+:" and found tons of these)
> the function definition says <variable>=None and then check if the
> variable is false then creates an empty var. In python empty vars are
> usually false, so if someones passes an empty var the function will
> create a new one, both creating more objects than it needs to and making
> it way harder to test openerp code. Changing it to if <variable> is None
> is both more semantically correct and will make the problem go away.
>

Well neither "not var" nor "var is None" can be a generally good solution for
all cases.
Sometimes we only want to test against the 'None' value, explicitly. Other
times we want to catch all negative values (such as False, [], {} and '').

I appreciate that there /might/ be places where the current "not var" test is
not the most politically correct.

But we can't just change all the instances blindly.

If you can indicate specific cases where the current test might be wrong, you
are welcome to help improve it.

[Leonardo Santagada @ Proge.com.br (santagada-proge)]

Every time I see:

def ...(... <var>=None ...):
  if not <var>:
    <var> = {} # or any other falsy mutable values like []

I see it as a bug, as you can't pass an empty mutable to the function (if you are interested on what/if any mutation happens in the function). It is also somewhat of a performance bug and a problem for reading code because in lot of places might call the function with different types, so while reading code you end up not knowing what is the correct api of a function.

I will try to find some time to patch some of the cases I see and send a pull request.

[Vo Minh Thu (thu)]

You are completely right.

I would rather have a specific bug than a general guideline (even if it is correct) but I will keep this bug report as wishlist to serve as a reminder.

[Cristian Salamea (ovnicraft)]

IMHO we need follow the rules from python, and make sure the value in var, in fact context must be context=None in all methods.

Will be difficult identify when if var: or if var is None:.

Regards,