Skip to content

Avoid using a mutable default value as an argument#13

Open
didibz wants to merge 2 commits intojondot:masterfrom
didibz:Avoid-Using-mutable-default-value-arguments
Open

Avoid using a mutable default value as an argument#13
didibz wants to merge 2 commits intojondot:masterfrom
didibz:Avoid-Using-mutable-default-value-arguments

Conversation

@didibz
Copy link

@didibz didibz commented Jun 17, 2020

The purpose of this change is to avoid using a mutable default value as an argument, since it's not good to do so in python.
This is an article that demonstrate the issue:
https://docs.quantifiedcode.com/python-anti-patterns/correctness/mutable_default_value_as_argument.html

@jondot
Copy link
Owner

jondot commented Jun 17, 2020

Thanks Didi
I would accept this if the parameters were actually mutated -- are they?
If so it could create interesting bugs

Otherwise what you're describing says -- what's the point in default params in Python, this feature is completely broken so let's never use it.

@didibz
Copy link
Author

didibz commented Jun 21, 2020

@jondot
In Python the default arguments are evaluated once when the function is defined, not each time the function is called. You should never define default arguments of mutable type unless you know what you are doing
The full article:
pythontips - Mutation

@didibz
Copy link
Author

didibz commented Jun 21, 2020

@jondot I agree that currently the arguments are not actually mutated. However, I see this more as good practice that removes a "trap" that may cause annoying bugs from future code changes that will mutate one of the arguments.

I think you can find the pattern on getting None instead of {}/[] as a default value in all popular Python open source libs (one example).

@didibz
Copy link
Author

didibz commented Jun 21, 2020

One last claim :) Is many cases you may pass the argument to another class/function/3rd-party (like here) that may mutate it. BTW, you can see that also CircuitBreaker uses this pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants