Category Archives: Heuristics library cleanup

Videos of the heuristics library refactor and cleanup.

Refactoring: Heuristics library episode 2 – Separation of concerns

In this video we discuss the separation of concerns and apply that to improving some of the code:

Summary

  • Discussed the separation of concerns concept
    • Improved code with this in mind
  • Set up an upstream repository and updated the fork of the repo to get new upstream changes
  • Made a PR with the changes in this episode: https://github.com/billy164/AAH/pull/4

If you have any questions please leave a comment!

Refactoring: Heuristics library episode 1 – introduction

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

Summary

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)]
    else:
        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.