We have a whole new refactoring series thanks to a library Billy has asked us to improve:


Here’s what happened in this video:

  • First look at code
  • Fixed mutable default parameter
  • Set up Tox for testing
    • Required writing a setup.py file
  • Set up TravisCI for continuous integration
  • Set up Coveralls for test coverage reports
  • Created PR with changes made in this video: https://github.com/billy164/AAH/pull/1

The issue with mutable default parameters

In the code there was a mutable default parameter:

def var_depth_search(number_of_machines, depth, number_of_tasks, tasks=[], limit=-1): 
    if not tasks:
        tasks = [random.randint(0, LONGEST_DURATION) for _ in range(number_of_tasks)]
        number_of_tasks = len(tasks)

In this particular case it works fine because tasks is rewritten every time as tasks never gets mutated but the overall pattern could have easily lead to issues.

For example if  tasks gets appended to or mutated in any way that adds values to it the default value will change and the else branch gets taken. This is a bit of a concern going forward, see this code for an example of what can happen:

>>> def var_depth_search(tasks=[]):
...     if not tasks:
...         tasks.extend([1,2,3])
...     else:
...         tasks.append("a")
...     print(tasks)
>>> var_depth_search()
[1, 2, 3]
>>> var_depth_search()
[1, 2, 3, 'a']
>>> var_depth_search()
[1, 2, 3, 'a', 'a']

You’d think if it was called with no arguments it should always go down the   if branch but that’s not what actually happens. Seeing as this was not the desired behavior I changed this code to check against  None to do the default parameter handling.