ADnD, Low Pass Filter and Extract Method
As I am very fond of old style Role Playing Games, I am working on a toy project about Advanced Dungeons and Dragons, 1st edition.
Now, this game is full of strange rules. For example, when the characters encounter a group of monsters, both parties have to roll a d6 (standard six faced die); if one of the parties rolls 1 or 2, that party is surprised. Also, in this case, the value of the die (1 or 2) indicates the number of "segments" (6 seconds period) for which that party is surprised.
In my first Python implementation, this lead me to write a method in the Party class, which was something like this:
Pretty simple, right?
This simple piece of code didn't satisfy me very much. The roll variable wasn't very useful here: it was just a temp variable to keep the roll value so that it could be used in the following expression.
Now, this game is full of strange rules. For example, when the characters encounter a group of monsters, both parties have to roll a d6 (standard six faced die); if one of the parties rolls 1 or 2, that party is surprised. Also, in this case, the value of the die (1 or 2) indicates the number of "segments" (6 seconds period) for which that party is surprised.
In my first Python implementation, this lead me to write a method in the Party class, which was something like this:
def surprise_segments(self):
roll = random.randint(1, 6)
return roll if roll <= 2 else 0
This simple piece of code didn't satisfy me very much. The roll variable wasn't very useful here: it was just a temp variable to keep the roll value so that it could be used in the following expression.
So, I decided to extract that part and see if I could come with something better:
This was much better, as now I just invoked the randint function without holding the result somewhere.
What was not satisfactory now was the name of the auxiliary function; it wasn't saying very much about the actual behavior of the code. Moreover, if I read "upper bound", I expect something which will cap at a specific value.
From the experiences of some previous (working) life, I remembered that I was implementing some sort of filter, a low pass filter ( https://en.wikipedia.org/wiki/Low-pass_filter ). In my case, this is an "Ideal low pass filter", but this was enough for me. So, now I could rename the function:
While I wasn't able (yet) to find other parts of my code which needed a low_pass_filter, I found similar patterns like:
So, this kind of pattern can be easily refactored:
There are some advantages in doing this:
There are a couple of refactorings which are relevant here:
def _upper_bound(self, roll):
return roll if roll <= 2 else 0
def surprise_segments(self):
return _upper_bound(random.randint(1, 6))
What was not satisfactory now was the name of the auxiliary function; it wasn't saying very much about the actual behavior of the code. Moreover, if I read "upper bound", I expect something which will cap at a specific value.
From the experiences of some previous (working) life, I remembered that I was implementing some sort of filter, a low pass filter ( https://en.wikipedia.org/wiki/Low-pass_filter ). In my case, this is an "Ideal low pass filter", but this was enough for me. So, now I could rename the function:
def _low_pass_filter(self, value, cutoff):
return value if value <= cutoff else 0
def surprise_segments(self):
return self._low_pass_filter(random.randint(1, 6), 2)
def a_method(self, ...)
x = something
y = something_else
do_something_with_x_and_y
So, this kind of pattern can be easily refactored:
def _do_something(self, x, y):
do_something_with_x_and_y
def a_method(self, ...)
self._do_something(something, something_else)
- a_method becomes shorter, which could make it easier to read
- do_something can get a better name which really explains what you are trying to do_something
- do_something is a behavior which can be re-used
- do_something is a behavior which can be stubbed/mocked in a test
There are a couple of refactorings which are relevant here:
Comments