lhenault/simpleAI

Considered bad practice to use mutable objects as kwargs

Nintorac opened this issue · 1 comments

Why it's bad - an initial call may alter the object and subsequent calls will have different results. see function a which has an ever increasing list and function c who's dictionary gets new entries on each call.

See example

>>> def a(b=[]):
...  print(b)
...  b+=[1]
... 
>>> a()
[]
>>> a()
[1]
>>> a()
[1, 1]
>>> a()
[1, 1, 1]
>>> a()
[1, 1, 1, 1]
>>> 
>>> 
>>> 
>>> def c(d={}):
...  d[len(d)]=1
...  print(d)
... 
>>> c()
{0: 1}
>>> c()
{0: 1, 1: 1}
>>> c()
{0: 1, 1: 1, 2: 1}
>>> c()
{0: 1, 1: 1, 2: 1, 3: 1}

reccomended workaround

def a(b=None):
  b = b or []

def c(d=None):
  d = d or {}

Though TBH the only time this has stung me is in an interview >.<

Yep I've learned about it the hard way once. The usual workaround isn't really working well with typing though, I see here the suggestion to rather use Optional:

def a(b=Optional[list]):

Another idea is to use Union[list, tuple] and have an empty tuple as default value. Accepting tuples is harmless and even potentially helpful in the few parts of the codebase I see the issue.