Just a bit more tidying, as Kent Beck puts it. Soon, new features or something. But today is not that day.

I couldn’t spend this much time refactoring and improving the code on a real product effort. What I’ve done here in a compact series of articles would have to take place over a more extended period of time, improving the code just as bit, ideally every day, but never requiring a big block of time that slowed down development of things that please the customer.

That said, I do think that we need to do code improvement all the time. The TDD rhythm is Red, Green, Refactor, not Red, Green, Back Away Slowly. And when browsing the code, and when we’re passing through some section of code, it behooves us to make some quick changes to improve things.

Tidying

I mentioned Kent Beck and “tidying” in the blurb. He is writing a new book, right in front of us, at the address linked here. You can sign up to support it and read it in progress. I’m finding it quite valuable, and recommend it to everyone who’s interested in producing valuable code in a sustainable fashion.

Committing Changes

If we look back over most of the improvements in this recent series of mine, we’ll see that while there are a few that required many changes to be complete, almost all of the improvements could be made in very tiny steps. That brings up the question of committing code. You’ll find folks who tell you to commit code improvements separately from features, or other notions of dividing up commits to make them meaningful.

I think that’s a good idea, but I wonder if it comes from the best place. I rather suspect that careful segregation of different kinds of changes into different commits comes from a place where we think we might have to back out some commits and want to be sure what we are losing if we do it.

That’s not where I live. My practice is always to commit forward. Sure, I’ll revert before committing. A time or two, I’ve backed out a commit or two in the Dungeon series. I don’t recall why, but it must have been a large change that I subsequently realized was bad. But even then, I don’t back one out and apply others. I roll back and then move forward.

You do you, and that certainly includes working out your own team’s commit strategy. But I do have a concern. If we set a rule that we commit code tidying and refactoring separate from “substantive” changes … when will we do the tidying. We may not be able to look at something, see a problem, fix the problem, and move on … because we’re doing a feature branch and therefore can’t do tidying.

I think that would be bad. I think that would lead to a code base that deteriorates faster the more pressure we feel. And the truth is, the more pressure we feel, the more we need to do tiny improvements all the time, because we’ll never “get time” for improvement any other way.

I’m not adept enough at git, nor have I worked with a team for a while. If you have good answers on this topic, let me know. Write an article and I’ll link to it, or even put it on my site. Help the world to know good things.

Me, I try always to move forward. When I move backward, it’s usually just the last step and I just revert rather than commit.

Anyway, today is about tidying, and other opportunities that may come up. Let’s look at the code and see what we can find.

Tidying Up the Dungeon

I started looking at GameRunner, because it’s usually full of things to improve, and I found this:

function GameRunner:createButtons()
    self.buttons = {}
    table.insert(self.buttons, Button("left",100,200, 64,64, asset.builtin.UI.Blue_Slider_Left))
    table.insert(self.buttons, Button("up",200,250, 64,64, asset.builtin.UI.Blue_Slider_Up))
    table.insert(self.buttons, Button("right",300,200, 64,64, asset.builtin.UI.Blue_Slider_Right))
    table.insert(self.buttons, Button("down",200,150, 64,64, asset.builtin.UI.Blue_Slider_Down))
    self:createCombatButtons(self.buttons)
end

function GameRunner:createCombatButtons(buttons)
    table.insert(buttons, Button:textButton("??", 100,350))
    table.insert(buttons, Button:textButton("History",200,350))
    table.insert(buttons, Button:textButton("Learn", 300, 350))
end

function GameRunner:drawButtons()
    for i,b in ipairs(self.buttons) do
        b:draw()
    end
end

I’m curious who calls the create.

function GameRunner:prepareToRun()
    self:createButtons()
    self:prepareCrawlAndTimers()
    self.playerCanMove = true
end

And the draw is surely called from somewhere in the drawing code. I don’t mind that, and I guess I don’t mind GameRunner owning the buttons: it kind of owns everything. But we could probably remove two methods from GameRunner if we can find another home for those two methods.

I kind of have to wonder why I decided to create those three “combat” buttons separately. Also they have nothing to do with combat, although I think I had that in mind at one point.

Where could we put this code if not in GameRunner? I guess the obvious choice is in DungeonBuilder. We’d have to return or set the buttons, though, because GameRunner needs to draw them.

Ron manages to avoid a bad idea.

Unless … this is surely not a great idea … unless we had GameRunner publish “timeToDraw” with suitable parameters, and things like Buttons would get the message. In general, though, drawing needs to be done in a certain order, and publish-subscribe has no particular notion of the order. Let’s file that idea but for now, assume that GameRunner will want to draw the buttons.

Who calls prepareToRun? Ah, GameRunner itself:

function GameRunner:createLevel(roomCount)
    local builder = self:defineDungeonBuilder()
    builder:buildLevel(roomCount)
    self:prepareToRun()
end

We could arrange that buildLevel creates the buttons. And that method could return the buttons … and be extended over time to return any other important items. I think we have a convention that we set things into GameRunner at this point, do we not? Yes:

function DungeonBuilder:placeWayDown()
    local r1 = self:getRooms()[1]
    local target = r1:centerTile()
    self.wayDownTile = self.dungeon:farawayOpenRoomTile(r1, target)
    local wayDown = WayDown(self.wayDownTile)
    self.RUNNER:setWayDown(wayDown)
end

We should stick to that convention for now, although returning a packet of information to GameRunner might be better. We’re not on that path right now, we’re planning to move the buttons.

We should create the buttons before dcFinalize, like this:

function DungeonBuilder:buildLevel(roomCount)
    self:dcPrepare()
    self:defineDungeonLayout(roomCount)
    self:placePlayer()
    self:customizeContents()
    self:createButtons()
    self:dcFinalize()
    return self.view:asFullDungeon()
end

If I test now, this will break. Let’s do.

DungeonBuilder:56: attempt to call a nil value (method 'createButtons')
stack traceback:
	DungeonBuilder:56: in method 'buildLevel'
	GameRunner:59: in method 'createLevel'
	Main:66: in function 'setup'

Implement a callback:

function DungeonBuilder:createButtons()
    self.RUNNER:createButtons()
end

And remove the call from GameRunner:

function GameRunner:prepareToRun()
    -- self:createButtons() -- deleted
    self:prepareCrawlAndTimers()
    self.playerCanMove = true
end

All is well. Commit: CreateButtons triggered in DungeonBuilder, calls back to GameRunner.

Now, still in GameRunner, change it to use a new setButtons method:

function GameRunner:createButtons()
    local buttons = {}
    table.insert(buttons, Button("left",100,200, 64,64, asset.builtin.UI.Blue_Slider_Left))
    table.insert(buttons, Button("up",200,250, 64,64, asset.builtin.UI.Blue_Slider_Up))
    table.insert(buttons, Button("right",300,200, 64,64, asset.builtin.UI.Blue_Slider_Right))
    table.insert(buttons, Button("down",200,150, 64,64, asset.builtin.UI.Blue_Slider_Down))
    self:createCombatButtons(buttons)
    self:setButtons(buttons)
end

function GameRunner:createCombatButtons(buttons)
    table.insert(buttons, Button:textButton("??", 100,350))
    table.insert(buttons, Button:textButton("History",200,350))
    table.insert(buttons, Button:textButton("Learn", 300, 350))
end

function GameRunner:setButtons(buttons)
    self.buttons = buttons
end

This should work. It does. Commit again: GameRunner:createButtons uses new setButtons accessor to set its buttons member.

Now let’s move the create methods over to DungeonBuilder:

function DungeonBuilder:createButtons()
    local buttons = {}
    table.insert(buttons, Button("left",100,200, 64,64, asset.builtin.UI.Blue_Slider_Left))
    table.insert(buttons, Button("up",200,250, 64,64, asset.builtin.UI.Blue_Slider_Up))
    table.insert(buttons, Button("right",300,200, 64,64, asset.builtin.UI.Blue_Slider_Right))
    table.insert(buttons, Button("down",200,150, 64,64, asset.builtin.UI.Blue_Slider_Down))
    self:createCombatButtons(buttons)
    self.RUNNER:setButtons(buttons)
end

function DungeonBuilder:createCombatButtons(buttons)
    table.insert(buttons, Button:textButton("??", 100,350))
    table.insert(buttons, Button:textButton("History",200,350))
    table.insert(buttons, Button:textButton("Learn", 300, 350))
end

I changed the last line of createButtons, and made them methods on DungeonBuilder rather than GameRunner. Should work perfectly, and does. Commit: DungeonBuilder now creates buttons, sets them into GameRunner.

Done. Three easy steps, no hassle. Took about ten minutes including the writing. So that’s nice. We’ve removed two building methods from GameRunner, making it that much more about running and less about building.

A Tiny Tidying

I noticed something small that I don’t like in GameRunner.

In Codea, I generally keep methods in alphabetic order, because it has no tree-like display of methods to make finding them easy. But I also have the habit, when I write a few methods at a time, of putting the subordinate methods adjacent to the most important one, to make it easier to copy and paste into these articles.

And, not to be outdone by my inconsistent brethren, sometimes I just group things together that seem to be related. The result of all this is that GameRunner is not particularly alphabetized.

So as a smaller tidying, I’m going to improve its alphabetization. Here’s a picture, from my Making app, of its current layout:

not very alphabetic

Now some manual sorting, since that’s all I’ve got to hand. Five minutes later, after testing, commit: Improve alpha order of GameRunner methods.

quite alphabetic

So that’s nice. Let’s sum up.

Summary

In well under an hour, including writing this article, we’ve moved two building methods out of GameRunner, and then tidied up GameRunner a bit more by quickly alphabetizing some methods. Commits were at 0856, 0900, 0903, and 0915.

And the code is a slightly better place to be. The campground is a bit cleaner. The code sparks a bit more joy. Pick your own metaphor, and, if you care to, think about what you can do in your own campground.

See you next time!