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.

I hope you’ll think about this and then do whatever seems right.

See you next time!