c2c-oerpscenario team mailing list archive

[Numérigraphe <525808@xxxxxxxxxxxxxxxxxx>]

Reporting progress on this issue, counting the occurrences of
"variable={}" in "def" lines :

Good news:
Addons: trunk has 350, down from 493 in 5.0
Web client: trunk has 29, down from 32 in 5.0

Bad news:
Server: trunk has 114, up from 100 in 5.0 (on the server! shame!)
Client: trunk has 70, up from 58 in 5.0
Extra: trunk has 868, up from 702 in 5.0

That means there is still a LOT of mindless coding and silly copy/paste going on.
That the partners didn't improve was expected - some clearly said they didn't give a damn - too bad.
But OpenERP SA said they would fix this over time, one function at a time as other improvements were being done. Obviously that doesn't work.

Dear maintainers, please either step in and have the coding practices improve at OpenERP SA, or consider again making a massive one-time fix across the whole code base. The script I wrote does not fix it all, but it sure will help.
Lionel.

-- 
Default values should be immutable
https://bugs.launchpad.net/bugs/525808
You received this bug notification because you are a member of C2C
OERPScenario, which is subscribed to the OpenERP Project Group.

Status in OpenObject Addons Modules: Confirmed
Status in OpenObject Addons extra-trunk series: New
Status in OpenObject Addons trunk series: Confirmed
Status in OpenObject GTK Client: Confirmed
Status in OpenObject Server: Confirmed
Status in OpenObject Server trunk series: Confirmed

Bug description:
Often in the code base we have:
def foo(...., context={},....):
    ....
As explained in comment #1, this should be changed to
def foo(...., context=None,....):
    # you should add this test only if the context is actually used
    if context is None:
        context={}
    ....

(This bug originally proposed the other way round hence the "nope" in comment #1.)

(xmo) Complements of information:

* The test should be `context is None` rather than `not context`: the caller could provide an empty dict argument {} and expect to have data in it after the call, if a test is done using `not context` an empty dict will match and the function will be creating a brand new context.
* The test should *only ever* performed when the current function fetches data from the context. If you don't do anything with the context but forward it to other functions and methods (e.g. read() or create()), no need to test for anything.