Today’s plan: Code review, and targets of opportunity. Short morning, just one new class.

I find value in looking back upon what I have wrought and seeing that it is good–or not so good. Actually, it’s the not so good where there’s learning to be had. In my view the learning is best when it leads to changes in practice, and the best way to do that, I think, is to put the idea into practice in the code we’re reviewing. With that in mind, let’s wander around a bit.

Looking at the Main tab, because it’s the one that comes up when you open the project, I see a sort of potpourri of functions. The list, in order, is a bit chaotic:

  • setup
  • startGame (local)
  • nextPlayersTurn
  • touched
  • draw
  • drawTouchToStart
  • drawButtons
  • rightButton
  • leftButton
  • drawSpeedIndicator
  • runTests
  • showTests
  • testsVisible
  • rectanglesIntersectAt
  • rectanglesIntersect

Now part of this chaos is due to the way I write code in general, and the way I write it when I’m doing these articles. Take the button-related code for example. I started with the drawButtons function, then extracted leftButton. I put that right below drawButton. It’s convenient to find when I want to glance at it, and it’s easy to copy and paste into the article.

Then when I wrote rightButton, I put it ahead of leftButton, because it calls leftButton. It looks like this:

function drawButtons()
    local y = Constant:saucerY() - 4
    
    leftButton("Touch for\nOnePlayer", 8, y-4)
    rightButton("Touch for\nTwo Players", 216, y-4)
end

function rightButton(s2, boxRightX, boxY)
    local margin = 8
    sx,sy = textSize(s2)
    leftButton(s2, boxRightX-margin-sx, boxY)
end

function leftButton(s1, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    local sx,sy = textSize(s1)
    fill(0,0,0,0)
    rect(boxX,boxY, 2*margin+sx,2*spacing+sy)
    fill(255)
    text(s1, textX, textY)
end

These three functions “belong together”, it seems to me. But there they are, right in between drawTouchToStart and drawSpeedIndicator, in inside-out non-alphabetical order. So while my placing of functions makes sense in the small–when I’m working–it’s not so good later on, for browsing or such.

Now in the case of the buttons, it seems clear to me that we have a little class coming into being, a button-drawing class. And if we pushed it a little further, we would pass touches to the button instances and let them answer whether they were touched. We might even give them enough information to actually do whatever is supposed to happen when they’re touched, rather than have someone else ask them if they’re touched and then do the thing. “Tell, don’t ask.”

You know what, having put this much thought into it, I think we should do that. We’ll make a Button class. I’ll start by leaving it right in Main, and move it soon.

Why, you ask? A few reasons. First, it reminds me of Keith Braithwaite’s “TDD as if you mean it” classes, where he teaches us to develop a class right in the test module and move it out later. It’s an interesting exercise.

Second, it keeps the user code and the class together in the same editor tab, which makes developing easier.

Third, same thing as before, it makes creating the article easier.

But I promise, we won’t leave our Button there forever.

Our button code now only has one user, the drawButtons function shown above, but if we’re going to make this into a class, we’re going to create buttons in setup and just draw and touch them elsewhere. We’ll see if we can evolve into that.

It’s worth noting that our touch code isn’t concerned with the buttons at all. It just interprets any touch on the left side as “one player” and on the right as “two players”. That gives us some time and freedom.

I’ll start by creating my buttons and drawing them every time around:

function drawButtons()
    local y = Constant:saucerY() - 4
    local b1 = Button:leftButton("Touch for\nOnePlayer", 8, y-4)
    local b2 = Button:rightButton("Touch for\nTwo Players", 216, y-4)
    b1:draw()
    b2:draw()
end

So far so good. We’re assuming two creation methods for two different styles of button. Now to slip these babies into a class. What about this:

Button = class();

function Button:rightButton(s2, boxRightX, boxY)
    local margin = 8
    sx,sy = textSize(s2)
    self:leftButton(s2, boxRightX-margin-sx, boxY)
end

function Button:leftButton(s1, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    local sx,sy = textSize(s1)
    fill(0,0,0,0)
    rect(boxX,boxY, 2*margin+sx,2*spacing+sy)
    fill(255)
    text(s1, textX, textY)
end

function Button:draw()
    
end

Notice that I’m not drawing anything, because, right now, the Buttons draw themselves as created. The code should still run, I think, but I’m wrong. The two methods don’t return a button, they return nil, so the draws don’t work. My clever idea is looking a little less clever. But …

Button = class();

function Button:rightButton(s2, boxRightX, boxY)
    local margin = 8
    sx,sy = textSize(s2)
    return self:leftButton(s2, boxRightX-margin-sx, boxY)
end

function Button:leftButton(s1, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    local sx,sy = textSize(s1)
    fill(0,0,0,0)
    rect(boxX,boxY, 2*margin+sx,2*spacing+sy)
    fill(255)
    text(s1, textX, textY)
    return Button()
end

function Button:draw()
    
end

Why am I doing it this way? I’m trying out an idea. Its main purpose is to keep the program working while I create this class, while using the class’s code. This is different from creating the class “outside” and then plugging it into drawButtons, and it’s different from breaking drawButtons for a longer time. (I admit it wouldn’t be very long. This is just a different way of working than usual, to see what happens.)

Now I need to create live instances that draw. It’s pretty clear what the draw needs to look like. It needs to look approximately like this:

    local sx,sy = textSize(s1)
    fill(0,0,0,0)
    rect(boxX,boxY, 2*margin+sx,2*spacing+sy)
    fill(255)
    text(s1, textX, textY)

Now the button’s string and coordinates never change. So all those numbers are constant for a given button. The two complex ones are really just the width and height of the box. Let’s change that in situ:

function Button:leftButton(s1, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    local sx,sy = textSize(s1)
    local boxW = 2*margin+sx
    local boxH = 2*spacing+sy
    fill(0,0,0,0)
    rect(boxX,boxY, boxW,boxH)
    fill(255)
    text(s1, textX, textY)
    return Button()
end

Still works fine. Now, right after we save boxH, we have all the parameters that the Button needs in order to draw. Therefore:

function Button:leftButton(s1, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    local sx,sy = textSize(s1)
    local boxW = 2*margin+sx
    local boxH = 2*spacing+sy
    return Button(s1,boxX,boxY,boxW,boxH,textX,textY)
end

function Button:draw()
    fill(0,0,0,0)
    rect(self.boxX,self.boxY, self.boxW,self.boxH)
    fill(255)
    text(self.message, self.textX, self.textY)
end

This ought to work if we define Button:init like this:

function Button:init(message, boxX, boxY, boxW,boxH, textX, textY)
    self.message = message
    self.boxX = boxX
    self.boxY = boxY
    self.boxW = boxW
    self.boxH = boxH
    self.textX = textX
    self.textY = textY
end

And in fact it does. We now have two self-drawing Button instances being drawn. We are, however, creating them every time we draw. That’s a bit wasteful (chronological duplication?) so we need to do something. There is, however, a weird concern. I discovered this by trying what I’m about to describe and finding that it did’t work.

You’d think I could move the creation of b1 and b2 (and thee defining of y) up into setup, and all would be well. You’d think that, but you’d be wrong.Here’s what happens when you do that:

boxes wrong

It takes me more fiddling than I want to admit to recognize that the creation depends on textSize and that depends on the current fontSize and we haven’t set that. Let’s do that, and extract:

local b1,b2

function setup()
    runTests()
    parameter.boolean("Cheat", false)
    Player1 = GameRunner(0,"Z")
    Player2 = Player1
    Runner = Player1
    defineButtons()
end

function defineButtons()
    local y = Constant:saucerY() - 4
    fontSize(10)
    b1 = Button:leftButton("Touch for\nOnePlayer", 8, y-4)
    b2 = Button:rightButton("Touch for\nTwo Players", 216, y-4)
end

And with our class, all is well:

Button = class()

function Button:init(message, boxX, boxY, boxW,boxH, textX, textY)
    self.message = message
    self.boxX = boxX
    self.boxY = boxY
    self.boxW = boxW
    self.boxH = boxH
    self.textX = textX
    self.textY = textY
end

function Button:rightButton(s2, boxRightX, boxY)
    local margin = 8
    sx,sy = textSize(s2)
    return self:leftButton(s2, boxRightX-margin-sx, boxY)
end

function Button:leftButton(s1, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    local sx,sy = textSize(s1)
    local boxW = 2*margin+sx
    local boxH = 2*spacing+sy
    return Button(s1,boxX,boxY,boxW,boxH,textX,textY)
end

function Button:draw()
    fill(0,0,0,0)
    rect(self.boxX,self.boxY, self.boxW,self.boxH)
    fill(255)
    text(self.message, self.textX, self.textY)
end

I think we need to protect the draw a bit more, and I’m not entirely happy with that b1,b2 thing. It’s not very general. I guess it’s better than a collection of two items. We’ll leave that. But we’d better do this:

function Button:draw()
    pushStyle()
    fontSize(10)
    fill(0,0,0,0)
    rect(self.boxX,self.boxY, self.boxW,self.boxH)
    fill(255)
    text(self.message, self.textX, self.textY)
    popStyle()
end

Should we make the font size a parameter to the button creation? Perhaps we should rather than requiring it to be setup from the outside. That’s temporal coupling and we don’t like it.

function defineButtons()
    local y = Constant:saucerY() - 4
    b1 = Button:leftButton("Touch for\nOnePlayer", 10, 8, y-4)
    b2 = Button:rightButton("Touch for\nTwo Players", 10, 216, y-4)
end

The change is pervasive in Button. I’m not at all sure that I like it:

function Button:init(message, size, boxX, boxY, boxW,boxH, textX, textY)
    self.message = message
    self.size = size
    self.boxX = boxX
    self.boxY = boxY
    self.boxW = boxW
    self.boxH = boxH
    self.textX = textX
    self.textY = textY
end

function Button:rightButton(s2, size, boxRightX, boxY)
    local margin = 8
    fontSize(size)
    sx,sy = textSize(s2)
    return self:leftButton(s2, size, boxRightX-margin-sx, boxY)
end

function Button:leftButton(s1, size, boxX, boxY)
    local margin = 8
    local spacing = 4
    local textX = boxX + margin
    local textY = boxY + spacing
    fontSize(size)
    local sx,sy = textSize(s1)
    local boxW = 2*margin+sx
    local boxH = 2*spacing+sy
    return Button(s1,size, boxX,boxY,boxW,boxH,textX,textY)
end

function Button:draw()
    pushStyle()
    fontSize(self.size)
    fill(0,0,0,0)
    rect(self.boxX,self.boxY, self.boxW,self.boxH)
    fill(255)
    text(self.message, self.textX, self.textY)
    popStyle()
end

That being said, our class works just fine, so it’s time to move it to its own tab and then commit: buttons drawn with Button class.

Reflection

I think it makes sense to move things like the buttons out of the Main tab, and into their own. Had we just moved the functions, we might not have known what tab to look in. But the implementation we have bothers me a bit. It’s not sufficiently dynamic, and the parameter list on init is rather immense. But no one is supposed to call init directly, only the left and right methods, so maybe that’s OK.

I’m still not entirely satisfied, but we did manage to get the class done while keeping things working almost all the time, except for the glitch with font size. I’ve been a bit cavalier with drawing settings. This tends to happen when I write “blob” functions like that, and it would be better to avoid them. At the same time, when you’re just adding “one more little thing”, it’s so easy to stuff it into the blob, which just gets blobbier and blobbier. (More blobby? Whatever.)

Still, it’s a righteous change. It’s also 11 AM, since I got a late start. I think we’ll wrap this up, and continue next time.

See you then!

Invaders.zip