A lesson from history, a surprise refactoring, and, apparently, a sermon.

A long time ago, around the turn of the century, I wrote a book called Extreme Programming Adventures in C#. It was published by Microsoft. At the time, Pearson, the publisher of Extreme Programming Installed, wanted the book but Microsoft outbid them substantially. It was the time of the recession, and Microsoft essentially gave me a year’s pay to write the book. Pearson told me “They’ll give you a lot of money, but they won’t sell the book”. That turned out to be the case, but at the time, the money mattered.

By the way, don’t look for a copy of the book to read: it’s out of date, although the story it tells is still valid. In it, I followed the development of a fairly powerful text editor, in my “warts and all style” that has followed me ever since, right down to this series.

But there was one special aspect to the story. At an early XP Immersion class, we had a guest student who was a certified software process assessor for CMI. At the end of the session, he told us two things. First, he said that he would certify any team that was doing XP at at least CMM Level 3.

Second, he said “but without up front design, some changes cannot be accommodated late in the development”. We didn’t agree and asked for an example. He thought and then said “You can’t put “undo” into a text editor if you haven’t planned for it from the beginning”.

So I wrote my text editor without catering for undo, although of course I knew it was coming up. But every design change I made was directed, not by future needs, other than the needs of the code now, or for the feature story I had in hand. And, at the end of the project, I “sprang” the undo requirement on myself.

It went in quite nicely, because well-factored code is as flexible as one could want.

I decided yesterday to spring a surprise on myself, and this time I had explicitly decided that I wouldn’t do it because it was mostly boring and tedious.

I’m going to convert this one-player game to have a two-player option. And we’ll see how hard–or how easy–it is.

How might we do this?

I happen to know how the original program handles two players. It has a table of all the individual player information, including the current images of their probably damaged shields. When it is time to switch players, they copy that player’s info into the game’s running table, and when it’s the other player’s turn, they copy the info back into the save table and copy the next player’s info in.

We don’t happen to have the same setup, so we can’t do that. But I have an idea for what we could do. In describing my plan, I want to back up a bit to compare how I’ve written this program compared to other ways it might be done.

It is common, for many programmers, whether playing with Codea or even working for a living, to write what we call “procedural” code. This is generally characterized by there being very few functions or subroutines, and there being long functions that carry out the various operations of the program sequentially, one after another, in whatever order made sense to the programmer at the time.

The original program is written somewhat in that style, as it was pretty common back when we were writing in assembler. Here’s a short example. Just read the comments:

0682: E1              POP     HL                  ; Pull data pointer from the stack (not going to use it)
0683: 3A 80 20        LD      A,(shotSync)        ; Sync flag (copied from GO-2's timer value)
0686: FE 02           CP      $02                 ; Are GO-2 and GO-3 idle?
0688: C0              RET     NZ                  ; No ... only one at a time
0689: 21 83 20        LD      HL,$2083            ; Time-till-saucer flag
068C: 7E              LD      A,(HL)              ; Is it time ...
068D: A7              AND     A                   ; ... for a saucer?
068E: CA 0F 05        JP      Z,$050F             ; No ... go process squiggly shot
0691: 3A 56 20        LD      A,(squShotStepCnt)  ; Is there a ...
0694: A7              AND     A                   ; ... squiggly shot going?
0695: C2 0F 05        JP      NZ,$050F            ; Yes ... go handle squiggly shot

0698: 23              INC     HL                  ; Saucer on screen flag
0699: 7E              LD      A,(HL)              ; (2084) Is the saucer ...
069A: A7              AND     A                   ; ... already on the screen?
069B: C2 AB 06        JP      NZ,$06AB            ; Yes ... go handle it
069E: 3A 82 20        LD      A,(numAliens)       ; Number of aliens remaining
06A1: FE 08           CP      $08                 ; Less than ...
06A3: DA 0F 05        JP      C,$050F             ; ... 8 ... no saucer
06A6: 36 01           LD      (HL),$01            ; (2084) The saucer is on the screen
06A8: CD 3C 07        CALL    $073C               ; Draw the flying saucer

The code jumps around, asking questions about random flags, saucers, squiggly shots, and so on. These things are not unrelated, but the structure isn’t broken out and ideas are inserted wherever in the code they might fit in. It is all like this, page after page.

Now, in fairness to whoever wrote the original code, there are plenty of functions in there, including some fairly general purpose ones. I’m not here to criticize decades-old assembler code by today’s higher-level language standards. I’m here to draw a comparison.

Often, programs like this one, which just grow as the developer adds new features, turn into long patches of code, reaching out more or less randomly to grab what they need.

Truth be told, this program that I’m working on still does more of that than I might like. Here’s a chunk of code I just found that’s hard to be proud of:

function Army:checkForKill(missile)
    for i, invader in ipairs(self.invaders) do
        if invader:killedBy(missile) then
            missile.v = 0
            return
        end
    end
    if self.rollingBomb:killedBy(missile) then
        missile.v = 0
        self.rollingBomb:explode(self)
    elseif self.plungerBomb:killedBy(missile) then
        missile.v = 0
        self.plungerBomb:explode(self)
    elseif self.squiggleBomb:killedBy(missile) then
        missile.v = 0
        self.squiggleBomb:explode(self)
    elseif self.saucer:killedBy(missile) then
        missile.v = 0
    end
end

What’s up with that duplication and if nest there at the end? Surely we can do better than that. Shall we try right now? I wasn’t planning to write any code this morning, just the introductory remarks but this cannot be allowed to stand.

Each of the bombs is an instance of Bomb class. We have some feature envy going on here, where we ask the bomb a question (will this missile kill you) and if the answer is yes, then we stop the missile (which isn’t even something we own), and tell the bomb to explode. When they explode, they do this:

function Bomb:explode(army)
    self.pos = self.pos - vec2(2,3)
    self.army = army
    self.explodeCount = 15
end

They remember the army as passed in, because later …

function Bomb:draw()
    pushStyle()
    if self.explodeCount > 0 then
        sprite(self.explosion, self.pos.x, self.pos.y)
        self.explodeCount = self.explodeCount - 1
        if self.explodeCount == 0 then self.army:deleteBomb(self) end
    else
        sprite(self.shapes[self.shape + 1],self.pos.x,self.pos.y)
    end
    popStyle()
end

… we call back to the army telling it that the bomb is gone (and it can drop another one). Just for completeness, we’ll look at that.

function Army:deleteBomb(aBomb)
    aBomb.alive = false
end

There’s no point to that being done by Army. The bomb could just as well mark itself not alive. I don’t remember whether we were doing something different, perhaps we were, or intended to instantiate new bombs, so we needed to give the army a chance to delete them. We no longer need that.

First refactoring, remove the deleteBomb, if we’re sure no one else uses it.

function Bomb:draw()
    pushStyle()
    if self.explodeCount > 0 then
        sprite(self.explosion, self.pos.x, self.pos.y)
        self.explodeCount = self.explodeCount - 1
        if self.explodeCount == 0 then self.alive = false end
    else
        sprite(self.shapes[self.shape + 1],self.pos.x,self.pos.y)
    end
    popStyle()
end

Game works fine, I can still kill bombs with missiles. Commit: remove Army:deleteBomb.

Now what about that if blob:

...
    if self.rollingBomb:killedBy(missile) then
        missile.v = 0
        self.rollingBomb:explode(self)
    elseif self.plungerBomb:killedBy(missile) then
        missile.v = 0
        self.plungerBomb:explode(self)
    elseif self.squiggleBomb:killedBy(missile) then
        missile.v = 0
        self.squiggleBomb:explode(self)
    elseif self.saucer:killedBy(missile) then
        missile.v = 0
    end

Couldn’t we write this:

function Bomb:processMissile(missile)
    if self:killedBy(missile) then
        missile.v = 0
        self:explode()
    end
end

function Bomb:explode()
    self.pos = self.pos - vec2(2,3)
    self.explodeCount = 15
end

(We don’t need the army parm in explode.)

And then do this:

function Army:checkForKill(missile)
    for i, invader in ipairs(self.invaders) do
        if invader:killedBy(missile) then
            missile.v = 0
            return
        end
    end
    self.rollingBomb:processMissile(missile)
    self.plungerBomb:processMissile(missile)
    self.squiggleBomb:processMissile(missile)
    if self.saucer:killedBy(missile) then
        missile.v = 0
    end
end

This is, of course, slightly less efficient, because we had elseif going on before. But it should work fine … and it does.

Commit: move bomb-missile handling more into Bomb from Army.

The saucer case is interesting. Why didn’t we explode the saucer, as we did with the bombs? Because it already knows to die if it’s hit:

function Saucer:killedBy(missile)
    if not self.alive then return false end
    if self:isHit(missile) then
        self:die()
        return true
    else
        return false
    end
end

Now if we move the missile kill into that function, we can remove that final if in Army.

function Saucer:killedBy(missile)
    if self.alive and self:isHit(missile) then
        self:die()
        missile.v = 0
    end
end

We don’t need to return the boolean any more. Now we can do this:

function Army:checkForKill(missile)
    for i, invader in ipairs(self.invaders) do
        if invader:killedBy(missile) then
            missile.v = 0
            return
        end
    end
    self.rollingBomb:processMissile(missile)
    self.plungerBomb:processMissile(missile)
    self.squiggleBomb:processMissile(missile)
    self.saucer:killedBy(missile)
end

Does this work? Yes, no surprise there. Commit: move saucer-missile handling to saucer.

Now I don’t like that those last four lines are all doing the same thing, but three call processMissile and one calls killedBy. Let’s rename the one in Saucer:

function Saucer:processMissile(missile)
    if self.alive and self:isHit(missile) then
        self:die()
        missile.v = 0
    end
end

function Army:checkForKill(missile)
    for i, invader in ipairs(self.invaders) do
        if invader:killedBy(missile) then
            missile.v = 0
            return
        end
    end
    self.rollingBomb:processMissile(missile)
    self.plungerBomb:processMissile(missile)
    self.squiggleBomb:processMissile(missile)
    self.saucer:processMissile(missile)
end

There. Nicer. Commit: rename Saucer:killedBy to processMissile.

Much better. We can perhaps learn from the way that Saucer handles it and come up with a nicer way for Bombs to do things, but for now I’m not motivated to do that.

There is, however, a kind of duplication going on here, in the way that the bombs and the saucer manage hits from missiles, which suggests that some kind of consolidation might be possible. Not for today, however. But I do want to comment on what just happened.

Objects Help with Structure

The changes we just made could be made in procedural code, or even in assembler. We could move little bits of assembly code to different places, removing duplication. And sometimes we used to do that, but it was tedious, not something you’d typically do just because you spotted an opportunity. You might, and you might get it right. But often, the opportunities are hard to spot in assembler, or in today’s more common high-level language procedural code.

In the case we just stumbled upon, we could easily see the if elseif elseif. We could easily see that the stuff inside the ifs was very similar. And we could even see that the code, in Army, was talking about things in Bomb. It became clear very easily that whatever was going on, it should be handled in Bomb rather than Army. And there was a place to move it, namely the Bomb class.

The way I program here, which I certainly didn’t invent but learned from my betters, results almost inevitably in visible problems with the code, and almost inevitably, there are better places to put things. Sometimes, we have to invent the place and then put things there. More often, as we go along, the places are already there.

You’ve seen things here go well. Today is an example. You’ve seen things here go badly. I am happy to take credit and blame for both, but the bulk of the credit for things going right is that the code itself is right. It has things that belong together placed together, and things that belong apart placed apart.

I do that more or less well, on a given day, depending on what I see, how I’m feeling, and possibly the phase of the moon. When I do manage to do it well, I’m following the advice and practices of masters who have gone before me, who have shown me some good ways to do things. I’d like to blame them for when things go badly, but none of them ever advised me to do things poorly, so I guess I’m stuck owning the nasty bits. But blame? No. Let me digress to talk about that.

Yes, if there’s a mistake in this code, I surely put it there. So, yes, it would be fair to “blame” me. That wouldn’t do us any good at all, though it might make me feel bad for a while. But not very long, because I have come to believe an odd thing very strongly:

On the day I write the worst code I’ve written in years, I am confident that I’m doing the best I could do on that day.

Not the best I could ever do. But that day, for some reason, I couldn’t draw out the energy, I couldn’t concentrate, I just couldn’t bring myself to try to figure out a better way or a better test. I was tired, or sick, or frustrated by politics, or distracted by my child’s illness, or something. On that day, and every day, I do the best I can.

Now, perhaps you have looked back on something you’ve done and hated yourself for it. Why was it necessary to eat the second bag of Doritos? Why did you have to make that nasty crack to someone you want to call friend? Why didn’t you drag your lazy um body to exercise? What a horrible person you are for that.

No, I really don’t think so. We are a complex mess of motivations, forces, energy, weaknesses. And we’re always doing the best we can muster. Why? Because every system always does the best it can. That’s what systems do. They just do their system thing.

We have free will, though, or at least we think we do. So, yes, we freely chose to eat that second bag of Doritos. Why? At the time it made the most sense we could make of the world.

That’s why I write these articles that show me making horrible stupid mistakes. Because some of you (not you, dear reader, I’m sure, but some of the other readers) some of you also make horrible stupid mistakes. And you may say to yourself “how do I work this?”. Well you work it by doing the best you can, and by trying to learn new better ways and develop new better habits.

Today, I committed the code many times, pretty much every time I made progress. I locked in that progress. Had things gone badly, I’d have been able to take just a small step back and be back in running order. That’s a habit I want to encourage.

Time to stop

I think I’ll wrap this up. I didn’t have in mind writing a sermon today, although it is Sunday. But I wrote one anyway.

I’m OK with that. I’ve been doing the best I can.

Your take home messages might be:

Our best

We’re doing our best. Our best isn’t the best we’ve ever done or the best we can imagine, it’s the best today. We can do better tomorrow … or in a few days … and that’s

Objects help

Objects make improving the code easier, because they keep common elements together and disparate elements apart.

Do they help enough?

When we start on the two-player game, we’ll be testing to see whether our objects are good enough to help us with that, and if they aren’t, what it takes to make them good enough.

See you next time!

Invaders.zip