[Erp5-dev] Default parameter (list) [eg] in OrderBuilder - which way is better?

[Łukasz Nowak]

Hello,

If list is passed as optional argument, its defaults shall be None or
[]?

Take for example ERP5/Document/OrderBuilder.py - everywhere foo_list on
method argument list has defaults to None. Without proper initialisation
there are problems, if method is called without changing parameter to
empty list eg.

Which is more correct:

def funA(self, movement_list=None):
if movement_list is None:
movement_list = []
=snip=

def funB(self, movement_list=[]):
=snip=

?

As I'm not passing movement_list to callAfterBuildingScript in
OrderBuilder (we are ongoing transition from 15401), this method dies
with exception.

Attached patch using funB method, only method declarations.

funA method allows caller to pass wrong argument type (None) while
invoking funA. Is it ok to do it without any warning?

Regards,
Luke
--
?ukasz Nowak R&D Ventis http://www.ventis.com.pl/
tel: [phone] fax: [phone]
``Use the Source, Luke...'' I am only craftsman.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OrderBuilder.py-change-default-parameters.patch
Type: text/x-patch
Size: 1669 bytes
Desc: not available
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080611/a939023f/attachment.bin>

[Mikolaj Antoszkiewicz]

Post by Łukasz Nowak
Hello,
If list is passed as optional argument, its defaults shall be None or
[]?
Take for example ERP5/Document/OrderBuilder.py - everywhere foo_list on
method argument list has defaults to None. Without proper initialisation
there are problems, if method is called without changing parameter to
empty list eg.
movement_list = []
=snip=
=snip=
?

In general, it's not desirable to use unmutables as default parameters.
They behave differently than usually expected to (consequent calls to
that function will work on existing list, not on a new empty one as one
might expect)
That's why funB type should be used only if You know what You're doing
and You want that.

Mikolaj

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3229 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080612/f6a9017e/attachment.bin>

[Łukasz Nowak]

Hello,

On 2008-06-12, 10:54:18

Post by Mikolaj Antoszkiewicz

Post by Łukasz Nowak
Hello,
If list is passed as optional argument, its defaults shall be None
or []?
Take for example ERP5/Document/OrderBuilder.py - everywhere
foo_list on method argument list has defaults to None. Without
proper initialisation there are problems, if method is called
without changing parameter to empty list eg.
movement_list = []
=snip=
=snip=
?

In general, it's not desirable to use unmutables as default
parameters. They behave differently than usually expected to
(consequent calls to that function will work on existing list, not on
a new empty one as one might expect)
That's why funB type should be used only if You know what You're
doing and You want that.

unmutable is immutable? list ([]) is not immutable, isn't it? So this
is not that issue? Or I got something wrong? (Now I'm quite confused.)

But ok. So the error, is that those variables wasn't changed to list if
None was passed?

Regards,
Luke

[Mikolaj Antoszkiewicz]

Post by Łukasz Nowak
Hello,
On 2008-06-12, 10:54:18

Post by Mikolaj Antoszkiewicz

Post by Łukasz Nowak
Hello,
If list is passed as optional argument, its defaults shall be None
or []?
Take for example ERP5/Document/OrderBuilder.py - everywhere
foo_list on method argument list has defaults to None. Without
proper initialisation there are problems, if method is called
without changing parameter to empty list eg.
movement_list = []
=snip=
=snip=
?

In general, it's not desirable to use unmutables as default
parameters. They behave differently than usually expected to
(consequent calls to that function will work on existing list, not on
a new empty one as one might expect)
That's why funB type should be used only if You know what You're
doing and You want that.

unmutable is immutable? list ([]) is not immutable, isn't it? So this
is not that issue? Or I got something wrong? (Now I'm quite confused.)

Arghh, sorry, sorry, my bad! Of course I meant MUTABLES and list is of
course mutable.

Mikolaj

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3229 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080612/cbd72dee/attachment.bin>

[Łukasz Nowak]

Hello,

On 2008-06-12, 11:35:13
Mikolaj Antoszkiewicz <mikolaj at erp5.pl> wrote:

(...)

Post by Mikolaj Antoszkiewicz
Arghh, sorry, sorry, my bad! Of course I meant MUTABLES and list

is

Post by Mikolaj Antoszkiewicz
of course mutable.

Having in mind what you said, I've created another patch, which is
initialising list in case of parameter passed as None.

Regards,
Luke
--
?ukasz Nowak R&D Ventis http://www.ventis.com.pl/
tel: [phone] fax: [phone]
``Use the Source, Luke...'' I am only craftsman.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OrderBuilder.py-initialise-list-to-empty-list-if-None.patch
Type: text/x-patch
Size: 840 bytes
Desc: not available
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080616/d5480adb/attachment.bin>

[Jérome Perrin]

Post by Łukasz Nowak
Hello,
On 2008-06-12, 11:35:13
(...)

Post by Mikolaj Antoszkiewicz
Arghh, sorry, sorry, my bad! Of course I meant MUTABLES and list

is

Post by Mikolaj Antoszkiewicz
of course mutable.

Having in mind what you said, I've created another patch, which is
initialising list in case of parameter passed as None.

Yes, this is the right way of doing.
Then question is: is this code just looking wrong or there's actually a
case where None is passed as movement_list ? in this case, the caller
might be wrong.

J?rome