Using a mutable object in api params can cause issues

[bucknerns]

https://github.com/braintree/braintree_python/blob/919b2fc87ff3865c9ddc986a48c170d2000d5c6f/braintree/credit_card_gateway.py#L20

` In [1]: def test(params={}): ...: print(params) ...: return params ...:

In [2]: resp = test()
{}

In [3]: resp["a"] = 1

In [4]: test()
{'a': 1} Out[4]: {'a': 1}

In [5]:
` If the params are returned and the user updates them it will cause the create this method and several others used throughout the code to remember the values.

There are several references to this https://docs.python-guide.org/writing/gotchas/.

[crookedneighbor]

The particular method you referenced doesn't matter, since it's not storing any state in the SDK, just passing those params onto the API call.

This could be the case for some of the configuration objects, but we allow global configuration changes anyway, so I'm not particularly concerned about it.

[ghost]

It is, however, poor coding practice and won't pass a simple pylint check:

test.py:1:0: W0102: Dangerous default value {} as argument (dangerous-default-value)

The fix is simple enough:

    def create(self, params=None):
        if params is None:
           params = {}

[bucknerns]

The point wasn't to call out a specific method it was to show a bad practice on a very important call, the one that handles credit card data. It should be fixed everywhere it's done like this to prevent the user from accidentally running into this issue.

Example usecase Func1 User performs action doesn't pass in params Response includes params User adds their own metadata to response params

Func2 User performs same action with no params Response contains metadata from func1 in params

[crookedneighbor]

I'm not disagreeing about whether or not it's best practice in general, just that the specific methods you've pointed out so far don't use the params beyond the API call, so there's no real danger here, and because of that, it's not a high priority for us at this time.

We will leave this issue open, and plan to get to this eventually, but right now we have higher priority things to work on.

If anyone would like to open a PR, we're happy to review it.

[sestevens]

This issue was resolved in version 4.1.0. Thanks, @maneeshd!