Refactoring: Heuristics library episode 1 – introduction

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.

2 Comments

  1. Thanks Janis for reviewing my code!

    Before I wrote the code for the heuristic, I found out that Python is dynamic typed. After reading more about different typing, I have a preference for static typing. Is there a nice way to avoid the mutable parameter issue whilst retaining static typing?

    1. The usual way to avoid the mutable parameter is to set the default as None (or some other immutable value) then check against that value. At that point (inside the function body) you can set the variable to your desired default value.

      Static typing means that you can make inferences about types at compile time. This notion is incompatible with how Python as a language works. Python is strongly typed but is dynamic (very dynamic). Because of the design of the language static typing of the sort found in Scala, Rust or C++ isn’t possible in Python. If you would like to have some sort of static analysis for types you can might want to look at type hinting. This along with tooling allows you to do some static analysis to perform type checking in your code. However this type hinting performs no type checking at runtime.

      The idiomatic thing in Python is to use Duck typing. Essentially if the passed parameter has the correct interface you should let it work. Generally speaking people expect duck typing with Python code, if you choose to do otherwise document why you are doing something different. This will save people reading your code having difficulties due to implicit assumptions about how your code works (see principle of least astonishment).

      If for some reason you absolutely must make sure that the parameter you are receiving will match an existing interface you do have a few options. If it has to be a specific type, and duck typing won’t do, the most basic thing you can do is to check for an explicit type with `isinstance`. This often is too specific, you usually only want to check if the type has a certain interface. If you need some specific interface have a look at abstract base classes (see PEP 3119 for an explanation of why this exists). In this case it would be something like `isinstance(tasks, Sequence)` to check we are getting a Sequence type. If you need to specify further because you are doing design by contract have a look at libraries such as zope.interface.

Leave a Reply

Your email address will not be published. Required fields are marked *