Python Asteroids on GitHub

Did you ever look at your code, man? Like, really look at it?

Yesterday, we worked on this code:

``````class Thumper:
def __init__(self, b1, b2):
self._long_interval = 30/60
self._short_interval = 8/60
self._decrement_interval = 127/60
self.b1 = b1
self.b2 = b2
self.reset()

# noinspection PyAttributeOutsideInit
def reset(self):
self._interval = self._long_interval
self._decrement_time = 0
self._execute_time = 0

# noinspection PyAttributeOutsideInit
def tick(self, delta_time):
self._execute_time += delta_time
if self._execute_time >= self._interval:
self._execute_time = 0
self.b1()
self.b1, self.b2 = self.b2, self.b1
self._decrement_time += delta_time
if self._decrement_time >= self._decrement_interval:
self._decrement_time = 0
self._interval = self._interval - 1/60
if self._interval < 8/60:
self._interval = 8/60
``````

Today, we’re going to look at this and other code. Like, really look at it.

Suppose that we didn’t know what this code did, and that we needed to change it. How might we work it out?

### In the before times …

What I used to do to understand code in the olden days, before we had things like the 32 inch monitor I’m looking at this morning, was to print out the code—we called it a listing—and I’d go over it with a pencil and draw lines on it, write words on it, trying to work out what the code did. I might scribble equations, whatever.

Once in a while, I’d grasp some section of it and I’d say, aloud or to my self, “I see what it’s doing here!”, and draw a circle or bracket around that code and a line off to a scribbled comment saying what that code did.

In the code above, obviously `tick` is where it’s at. Right away, we might notice the two independent bits of the code. We might draw a line between them:

``````    def tick(self, delta_time):
self._execute_time += delta_time
if self._execute_time >= self._interval:
self._execute_time = 0
self.b1()
self.b1, self.b2 = self.b2, self.b1
# -------------------------------------------
self._decrement_time += delta_time
if self._decrement_time >= self._decrement_interval:
self._decrement_time = 0
self._interval = self._interval - 1/60
if self._interval < 8/60:
self._interval = 8/60
``````

We’d look further. We’d see, after a bit of thinking that `_execute_time` counts upward until it is larger than `_interval`, then gets set down to zero and the code calls `b1`.

We read the next line, we see that it is swapping `b1` and `b2`. We noodle just a bit and say, “I see. Next time it will call `b2`, and then go back to `b1`. It alternates!”

Summing up our learning, we might say “This first bit alternately calls `b1` and `b2`, every `_interval` seconds.”

Similar analysis would lead us to something like “Every `_decrement_interval` seconds, it reduces the `_interval` between calls by one sixtieth of a second. It makes the calls happen faster and faster!”

We’re smart people. We do that sort of thing every day.

### In the today times …

Yesterday, I refactored that code, as you may recall, and today it looks like this:

``````    def tick(self, delta_time):
if self.it_is_time_to_beat(delta_time):
self.play_and_reset_beat()
if self.it_is_time_to_speed_up_beats(delta_time):
self.speed_up_beats()

def it_is_time_to_beat(self, delta_time):
self._time_since_last_beat += delta_time
return self._time_since_last_beat >= self._time_between_beats

def play_and_reset_beat(self):
self._time_since_last_beat = 0
self.b1()
self.b1, self.b2 = self.b2, self.b1

def it_is_time_to_speed_up_beats(self, delta_time):
self._time_since_last_decrement += delta_time
return self._time_since_last_decrement >= self._delay_before_shortening_beat_time

def speed_up_beats(self):
self._time_since_last_decrement = 0
self._time_between_beats = max(
self._time_between_beats - self._amount_to_shorten_beat_time,
self._shortest_time_between_beats)
``````

There were a fair number of refactoring steps in that transformation. There are four extracts and around six renames. It took me less time to do than it took to write the description of how it works up above.

Now suppose we want to understand this code. We look at this:

``````    def tick(self, delta_time):
if self.it_is_time_to_beat(delta_time):
self.play_and_reset_beat()
if self.it_is_time_to_speed_up_beats(delta_time):
self.speed_up_beats()
``````

What does that code do? Well, heck, we don’t need no thinkin’ here, do we? If it’s time to beat, we play and reset the beat, and if it’s time to speed up the beats, we speed up the beats.

If our job was to add a third beat, we’d know right now to look at `play_and_reset_beat`, and if our job was to slow down the way the beats speed up, we’d know to look at `speed_up_beats`.

Suppose we do look at the speeding-up code. In the old form it was:

``````        self._decrement_time += delta_time
if self._decrement_time >= self._decrement_interval:
self._decrement_time = 0
self._interval = self._interval - 1/60
if self._interval < 8/60:
self._interval = 8/60
``````

And in the new form:

``````    def speed_up_beats(self):
self._time_since_last_decrement = 0
self._time_between_beats = max(
self._time_between_beats - self._amount_to_shorten_beat_time,
self._shortest_time_between_beats)
``````

We shorten the time between beats by `_amount_to_shorten_beat_time`. We Command+B to see what that is and find, in init:

``````        self._amount_to_shorten_beat_time = 1 / 60
``````

We change the number to something smaller and there we are, done.

I hope you can see that the second form of `tick` is vastly easier to comprehend than the first one. And I hope you remember that in some of the code your team faces, the variable names might not even have been as “clear” as `_decrement_interval`, whatever the heck that means.

Is it worth it to refactor the original code into the current form? I guess if it was hard to write it should be hard to understand? I need to understand this object because I’m going to turn it into a Flyer, but it seems to me that the extra effort to do a job that’s about this good is pretty low.

In fact, it wouldn’t be that difficult to have programmed it that way initially. It’s basically just an outline of what has to be done:

``````    def tick(self, delta_time):
if self.it_is_time_to_beat(delta_time):
self.play_and_reset_beat()
if self.it_is_time_to_speed_up_beats(delta_time):
self.speed_up_beats()
``````

I could have written it that way to begin with and then filled in the blanks. I learned that technique ages ago, from Kent Beck, calling it “programming by intention”, where you write what you intend for the code to do, and then make it do what you intend.

I have fallen into the habit of programming by intention less and less, and I’m not sure why. I’ll try to do more of it.

One more thing. I think this code could be even better. The names `b1` and `b2` aren’t very helpful. Let’s rename:

``````class Thumper:
def __init__(self, first_action=None, second_action=None):
self._longest_time_between_beats = 30 / 60
self._shortest_time_between_beats = 8 / 60
self._delay_before_shortening_beat_time = 127 / 60
self._amount_to_shorten_beat_time = 1 / 60
self._time_between_beats = 0
self._time_since_last_beat = 0
self._time_since_last_decrement = 0
self.current_action = first_action if first_action else self.play_beat_1
self.next_action = second_action if second_action else self.play_beat_2
self.reset()

@staticmethod
def play_beat_1():
player.play("beat1")

@staticmethod
def play_beat_2():
player.play("beat2")

def play_and_reset_beat(self):
self._time_since_last_beat = 0
self.current_action()
self.current_action, self.next_action = self.next_action, self.current_action
``````

Better? I think so. Test and commit: rename variables.

It’s important to commit very often when doing this kind of thing. Once in a while, I forget, as I did the first time through here, and then what I did next I didn’t like and I had to redo it. Each time we get to a stable point, we should save the game.

There are more things that could make this code even better (according to my standards of better). But let’s stop here and sum up.

## Summary

A few simple renames and extracting some methods with helpful names can make a big difference in how quickly we can understand code when we set out to change it. In my view, it’s always worth a little bit of cleanup when we “finish” a thing, and don’t forget the value in refactoring for understanding.

### Refactoring for understanding???

Remember that thing where we identify a block of code and say “I see what it’s doing here!!!”

Here’s some randomly selected code:

``````    def perform_interactions(self):
self.fleets.begin_interactions()
for target, attacker in itertools.combinations(self.fleets.all_objects, 2):
self.interact_one_pair(target, attacker)
self.fleets.end_interactions()
``````

Now this is pretty simple and easy to understand. But that bit in the middle is a bit much. What is it? Oh, right, I see that it’s interacting all pairs of objects. So we extract a method:

``````    def perform_interactions(self):
self.fleets.begin_interactions()
self.interact_all_pairs()
self.fleets.end_interactions()

def interact_all_pairs(self):
for target, attacker in itertools.combinations(self.fleets.all_objects, 2):
self.interact_one_pair(target, attacker)
``````

That’s refactoring for understanding. Grok a thing, pull it out, give it a name.

To do it took typing Option+Command+M and the name. A few keystrokes and we have recorded our understanding forever.

Is that itertools thing obscure? You betcha. We could refactor that for understanding, too:

``````    def interact_all_pairs(self):
for target, attacker in self.all_pairs():
self.interact_one_pair(target, attacker)

def all_pairs(self):
return itertools.combinations(self.fleets.all_objects, 2)
``````

It’s that easy. It pays off now, because I feel good about making the code more clear (by my standards), and it pays off later when I have to review this code or change it. Quite often, I can just review the top level and move on:

How does `perform_interactions` work?

``````    def perform_interactions(self):
self.fleets.begin_interactions()
self.interact_all_pairs()
self.fleets.end_interactions()
``````

Oh right: It just does the `begin`, interacts all the pairs, and does the `end`.

Our job is hard enough, but it’s easy to make it a bit easier with a few renames and extracts.

I’m just here to show you what I do, what happens, and what I think about what happens, but if I did give advice, I’d … but no.