Probably my last article of 2020. Some thoughts on these articles, and the design of OO programs. Perhaps a character sheet for the Player.

Right or Wrong, It’s In Here

I try, in these articles, to be open about the things that go wrong, which is to say, to be open about the mistakes I make. I do that for a few reasons:

Entertainment
It entertains me to write these programs and articles, and I a perpetually amused by what a wide range of mistakes I make, despite my decades of experience programming. Sometimes a mistake will be a well-considered decision that just turns out not to work out. More commonly, my mistakes seem to spawn and thrive in parts of the code that are more messy or otherwise not as good as they might be. And, quite commonly, I just make trivial mistakes that you’d think one of the million monkeys typing Shakespeare could have avoided.
It entertains me, and I hope it entertains you.
Permission
I show readers these mistakes to grant permission for everyone to make mistakes, and to look with surprise at them, and to think about what is contributing to their mistakes, with an eye to avoiding some of them.
Not that anyone needs permission from me, but maybe seeing that I can survive showing my mistakes will help folks look at their work a bit less defensively, and therefore a bit more productively.
Learning
I write these articles as I program because it helps me learn as I work. It’s certainly a good idea to look back every week, or every day, to see what one might learn from what has happened. But I find that even an hour after something has gone wrong, my learning is a bit fuzzier, my ideas for what to do more general. Reflecting on the work as I go seems to help me learn.
Perhaps you’d have the same experience.
Ready to Ship
Working this way causes me to work in very small steps. As I practice choosing small steps forward and implementing them, I am able to keep the program “ready to ship”. Oh, it certainly doesn’t have every feature that we might want, but whatever has been built so far looks like the product we’re working on, and with rare exceptions, everything in it works.
If I were condemned to return to some of my less successful product efforts in the past, and could make only one change to what I did, I’d keep the work ready to ship. I think that would have turned some pretty big failures into successes.
Learning
One very good thing about small steps is that when they don’t work out, it’s easy to reverse and choose a new direction. Even long after the small chunk has been put in place, the nature of small chunks makes it likely that we can improve or replace anything without mass changes all over the program.
It’s far better to be able to fold our learning into the system we’re working on now, rather than into the one we may be working on months or years from now.

Impact on Design

Working this way, in small steps, reflecting constantly, tends to lead me to a style of design that I find works well, and that aligns well with the design thinking of the many geniuses in whose steps I stumble along. These include:

High Cohesion
Cohesion is the notion that a programming element, a statement, a block, a method, a function, an object, a module ought to hang together–to cohere. It should be about “one thing”, and everything about that one thing should be in this element, and nothing not about that one thing should be in this element.
I’m not good enough to get to perfect cohesion, if it’s even possible. But I discover repeatedly that the closer I get to each programming element being about a single notion, the better things seem to go.
Cohesion should be high.
Low Coupling
Coupling is the notion referring to how many “other things” a given programming element thinks about, how many unique kinds of things it deals with or talks to. Things seem to go better when coupling is kept low, and to go worse when systems are highly coupled. Changes are easier with low coupling. Errors increase with high coupling.
It’s not clear what the right coupling number is. It seems that zero would be useless, and one or even two seems often to be impossible. Larger numbers become more and more questionable.
Coupling should be low.
Law of Demeter
Someone expressed the Law of Demeter as “You can put your hands in your pockets, but you shouldn’t put your hands in your friends’ pockets”. More formally, it’s often expressed by saying that an object method can send messages to its parameters, and to the object’s member variables, but shouldn’t reach beyond those. We’ll look at a violation of that law in the Dungeon game later on.
The “strong suggestion of Demeter” tends to keep cohesion high and coupling low. Those are good things.
Tell, Don’t Ask.
This is the notion that instead of asking an object a question, and then doing something with the answer, we should tell the object to do something on our behalf. When we ask something and then use the result, we tend to get duplicated code spread around the system, and that code is likely to be coupled to details that would be better off embedded int he object we spoke to.
When we tell the object to do things on our behalf, coupling tends to decrease and cohesion tends to increase.
Small, Even Tiny Methods
When we work toward the ideas above, we tend to get smaller methods. Where we might have had a few lines using values from an object we know, we can wind up with just one line telling that object what to do. And that method will be just a few lines, by the nature of what we just did.
Long methods have to be studied to be understood. Small methods are easy to inspect. Tiny methods are generally obvious.

In Practice

In practice, I manage to follow the above ideas, um, well, pretty well sometimes. Sometimes even very well. Often, not so well at all. Why is that?

One possibility is that I’m just no damn good. I reject that notion on the grounds that if it were true I should have given up a long time ago, and now I’d have nothing to fill my time.

Another possibility, one that I would like to believe, is that I fail to follow those rules because I am good at what I do. I can envision a large design and get it nearly right. I can write long patches of procedural code and get them nearly right. Even now, in what some argue is my dotage, I can do those things. Imagine when I was as young as you are. Wow.

However, we also see code that doesn’t follow those ideas from very inexperienced programmers, who are not as good as I am, much less as good as you are. It seems that almost everyone veers away from these valuable idea quite often. And those who never do? Gods and goddesses among humans, I guess.

I’ll do better to assume that they are not special in some essential way that I can never aspire to. Kent Beck said:

I’m not a great programmer; I’m just a good programmer with great habits.

That sounds possible, Probably I can build up great habits too, inch by inch, step by step1.

And that’s what all these articles are about, exploring my own habits and trying to improve them. As faithful readers will have seen, some of them I improve fairly easily, and sustain for a while, and other habits seem to fade away quickly. I keep forgetting to commit my code. I keep rationalizing now writing tests, even when I’d probably do better not to.

Maybe I’m a bad person after all. No, just human.

To build up good habits in practice … we have to practice.

Now let’s look at some code.

The Code, in Practice

I promised to show you a violation of the Law of Demeter, so let’s start with that.

function draw()
    ...
    if DisplayToggle then
        local center = Runner.player:graphicCorner()
        focus(center, 1)
    else
        scale(0.25)
    end
    ...
end

That right there is a violation of the LoD, though arguably only a small one. The Main does know the Runner, so it is OK to talk to it. But instead, Main grabs one of Runner’s member variables, player, and sends a message to it. Then it uses that value to set the focus.

It would be more reasonable, according to LoD and “Tell, Don’t Ask”, to ask Runner to focusOnPlayer and move the focus function into GameRunner. When we look at focus, things only get worse:

function focus(center, zoom)
    local LOWX,LOWY = maxScrollValues()
    translate(clamp(LOWX, WIDTH/2-center.x, 0), clamp(LOWY, HEIGHT/2-center.y, 0))
end

function maxScrollValues()
    local DW,DH = Runner:dungeonSize()
    return WIDTH - DW, HEIGHT - DH
end

We’re scarfing up other values from Runner, the width and height of the dungeon. And we use those values to compute a translation of the screen.

We actually use this function twice in main. It’s also used here:

function drawTinyMap()
    if not DisplayToggle then return end
    local sc = 0.05
    pushMatrix()
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    DisplayToggle = false
    scale(sc)
    Runner:draw(true)
    DisplayToggle = true
    popMatrix()
end

Now this code is messy enough for other reasons, including the notion of “DisplayToggle”. That’s used all the way down in Tile:

function Tile:getLargeScaleTint()
    if not DisplayToggle then return color(255)  end
    return self.tintColor
end

This use of a global is arguably also a Law of Demeter violation. We’re jamming a fact down into Tile from Main. We’d do better to pass it as a parameter, and better still if we could express what needs to be done without a boolean at all.

Hmm …

I wasn’t really planning to do this, but maybe we should clean up this code to get an idea of what a better version would look like, and to see how difficult it is to make it better. With any luck, it’ll be easy enough to make it clear that it’s worth doing.

OK, let’s do it.

The first thing to remember is that the game originally had the ability to display the whole map, zoomed out just enough so that it would all fit on the screen. That ability is now disabled, but it’s still there.

map

All I had to do to make that happen was to make this

function touched(aTouch)
    --[[
    if aTouch.state == ENDED then
        DisplayToggle = not DisplayToggle
        TouchX = aTouch.pos.x
        TouchY = aTouch.pos.y
    end
    ]]--
    Runner:touched(aTouch)
end

Into this:

function touched(aTouch)
    --[[]]--
    if aTouch.state == ENDED then
        DisplayToggle = not DisplayToggle
        TouchX = aTouch.pos.x
        TouchY = aTouch.pos.y
    end
    --]]--
    --Runner:touched(aTouch)
end

I’d really hate to lose that ability entirely. It was useful in debugging earlier on, and it might be useful again. So as we refactor this, let’s preserve the ability to draw that map, and let’s even consider a secret keystroke to display it.

Thinking about the top level display, we see three versions of the map that can be displayed:

  1. The zoomed-in one that shows the large view of the player’s immediate environment;
  2. The zoomed-out one that is the tiny growing map in the top right corner.
  3. The zoomed-out one that fills the screen with the whole maze;

The default behavior of the system is to display numbers one and two always. There’s no way to make it display number 3 at all, without patching the code.

The flow of drawing is this:

    Runner:draw(false)
    popMatrix()
    Runner:drawButtons()
    drawTinyMap()

This happens after the Main has decided what scale to use, and presently it always uses the “focused” view that draws number one. Then it turns off the scaling to draw the buttons and the tiny map.

It seems to me that the GameRunner would do well to have all the code for drawing the game. (However, GameRunner is getting large, and we need to consider whether it needs breaking up. Be that as it may, drawing the game makes sense in GameRunner, at least to me.)

So let’s move all this into GameRunner:

function draw()
    Runner:draw()
end

function GameRunner:draw()
    font("Optima-BoldItalic")
    pushMatrix()
    if CodeaUnit then showCodeaUnitTests() end
    if DisplayToggle then
        local center = self.player:graphicCorner()
        focus(center, 1)
    else
        scale(0.25)
    end
    self:drawMap(false)
    popMatrix()
    self:drawButtons()
    self:drawTinyMap()
end

function GameRunner:drawTinyMap()
    local sc = 0.05
    pushMatrix()
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    DisplayToggle = false
    scale(sc)
    Runner:drawMap(true)
    DisplayToggle = true
    popMatrix()
end

Basically I just blindly moved the draw and drawTinyMap into GameRunner. To allow that, I renamed the existing GameRunner draw to drawMap:

function GameRunner:drawMap(tiny)
    fill(0)
    stroke(255)
    strokeWidth(1)
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:draw(tiny)
        end
    end
    self.player:draw(tiny)
end

I removed a check for DisplayToggle from the beginning of drawTinyMap.

We’re far from done here but the game runs. Commit: moving all drawing to GameRunner.

Now let’s do some cleaning up, starting with DisplayToggle. We’re using that in two ways. One was the original purpose, to determine whether to display our current normal double map, or the zoomed one that shows the whole dungeon. We can’t do that at all now, and let’s just let it be.

function GameRunner:draw()
    font("Optima-BoldItalic")
    pushMatrix()
    if CodeaUnit then showCodeaUnitTests() end
    if DisplayToggle then
        local center = self.player:graphicCorner()
        focus(center, 1)
    else
        scale(0.25)
    end
    self:drawMap(false)
    popMatrix()
    self:drawButtons()
    self:drawTinyMap()
end

We really should move the CodeaUnit bit back to Main:

function draw()
    if CodeaUnit then showCodeaUnitTests() end
    Runner:draw()
end

Now the use of DisplayToggle there is to decide whether to scale for maps 1 and 2, and we want always to do that. Let’s move that into GameRunner while we’re at it.

First this:

function GameRunner:draw()
    font("Optima-BoldItalic")
    pushMatrix()
    self:scaleForLocalMap()
    self:drawMap(false)
    popMatrix()
    self:drawButtons()
    self:drawTinyMap()
end

function GameRunner:scaleForLocalMap()
    if DisplayToggle then
        local center = self.player:graphicCorner()
        focus(center, 1)
    else
        scale(0.25)
    end
end

Then just ditch the DisplayToggle stuff and move focus:

function GameRunner:scaleForLocalMap()
    local center = self.player:graphicCorner()
    self:focus(center, 1)
end

function GameRunner:focus(center, zoom)
    local LOWX,LOWY = self:maxScrollValues()
    translate(clamp(LOWX, WIDTH/2-center.x, 0), clamp(LOWY, HEIGHT/2-center.y, 0))
end

function GameRunner:maxScrollValues()
    local DW,DH = self:dungeonSize()
    return WIDTH - DW, HEIGHT - DH
end

Game works. Alphabetize methods, then commit: moved focus code into GameRunner.

Since I decided not to make clamp into a GameRunner function, I moved it back to Main as a free-standing potentially useful math function.

Commit: move clamp to Main.

Now let’s see what else is going on with DisplayToggle.

function Tile:getLargeScaleTint()
    if not DisplayToggle then return color(255)  end
    return self.tintColor
end

This is a bit weird. tintColor is set based on distance from the player. It’s the function that makes the big map get darker as the radius increases. If we remove that check for DisplayToggle right now, I think the tiny map will be colored incorrectly. No, that’s not the case. The flag only deals with t zoomed to fill screen map. I’m not catering to that, so I can delete that line.

function Tile:getLargeScaleTint()
    return self.tintColor
end

The only remaining usage in GameRunner is here:

function GameRunner:drawTinyMap()
    local sc = 0.05
    pushMatrix()
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    DisplayToggle = false
    scale(sc)
    Runner:drawMap(true)
    DisplayToggle = true
    popMatrix()
end

No one here is looking at that flag, so I think I can remove those two lines.

function GameRunner:drawTinyMap()
    local sc = 0.05
    pushMatrix()
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
    Runner:drawMap(true)
    popMatrix()
end

Game plays fine. Commit: DisplayToggle removed from GameRunner.

I think there are no references to DisplayToggle anywhere but in Main and the tests. So the tests should be able to remove theirs, since they condition nothing.

Tests run. Commit: DisplayToggle removed from Tests.

Clearly no one care now. Remove from Main, and so commit. I also removed the commented-out scaling code for the large full-map display. Commit: DisplayToggle removed. No way to display full map.

Main is now pretty simple:

function setup()
    if CodeaUnit then 
        codeaTestsVisible(true)
        runCodeaUnitTests() 
    end
    local seed = math.random(1234567)
    print("seed=",seed)
    math.randomseed(seed)
    showKeyboard()
    TileLock = false
    Runner = GameRunner()
    Runner:createLevel(12)
    --Runner:createTestRooms()
    TileLock = true
end

function draw()
    if CodeaUnit then showCodeaUnitTests() end
    Runner:draw()
end

function keyboard(key)
    Runner:keyPress(key)
end

function touched(aTouch)
    Runner:touched(aTouch)
end

function clamp(lo,val,hi)
    if lo <= hi then
        return math.max(math.min(val,hi), lo)
    else
        return math.max(math.min(val,lo), hi)
    end
end

And the logic is overall far more clear, because we no longer have to worry about what that darn DisplayToggle may be doing.

GameRunner is getting large, though. It has 29 methods. They are broken down, roughly, into two different notions, creating the level map, and drawing the level map. This is manifestly not as cohesive as it could be. The code is telling us that there want to be at least two objects here, one doing the setup, and one doing the drawing. That is something worth considering, and longer term it could have additional value in that it might facilitate creating maps differently, or drawing them differently, with these two notions separated.

For now, I don’t think I’ll go after that. Let’s do take a summary look at the drawing part, which is what we just worked on, and assess it according to whatever design standards come to mind.

function GameRunner:draw()
    font("Optima-BoldItalic")
    pushMatrix()
    self:scaleForLocalMap()
    self:drawMap(false)
    popMatrix()
    self:drawButtons()
    self:drawTinyMap()
end

This is nearly good, but the pushing and popping is a bit odd. Let’s see if this is better:

function GameRunner:draw()
    font("Optima-BoldItalic")
    self:drawLargeMap()
    self:drawButtons()
    self:drawTinyMap()
end

function GameRunner:drawLargeMap()
    pushMatrix()
    self:scaleForLocalMap()
    self:drawMap(false)
    popMatrix()
end

function GameRunner:drawTinyMap()
    local sc = 0.05
    pushMatrix()
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
    Runner:drawMap(true)
    popMatrix()
end

That seems nearly good. I’d like to pull the pushMatrix up one line in that last method:

function GameRunner:drawTinyMap()
    pushMatrix()
    local sc = 0.05
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
    Runner:drawMap(true)
    popMatrix()
end

I think in general I like the pushes and pops at the outer edges of the methods, like parentheses.

These methods seem nice to me. They are about as short as they can be, and they do very little thinking. We could perhaps improve drawTinyMap this way:

function GameRunner:drawTinyMap()
    pushMatrix()
    self:scaleForTinyMap()
    Runner:drawMap(true)
    popMatrix()
end

function GameRunner:scaleForTinyMap()
    local sc = 0.05
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
end

That separate out the doing bits from the bits that just call for things to be done. Nice.

Comparing these two methods:

function GameRunner:scaleForLocalMap()
    local center = self.player:graphicCorner()
    self:focus(center, 1)
end

function GameRunner:scaleForTinyMap()
    local sc = 0.05
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
end

We see that the one defers part of its calculation and the other does not. Let’s inline focus:

function GameRunner:scaleForLocalMap()
    local center = self.player:graphicCorner()
    local LOWX,LOWY = self:maxScrollValues()
    translate(clamp(LOWX, WIDTH/2-center.x, 0), clamp(LOWY, HEIGHT/2-center.y, 0))
end

function GameRunner:scaleForTinyMap()
    local sc = 0.05
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
end

Now they look a bit more alike. I think I prefer that. I notice that the Local doesn’t scale: it operates at scale 1. Should we say that? Is this more clear?

function GameRunner:scaleForLocalMap()
    local center = self.player:graphicCorner()
    local LOWX,LOWY = self:maxScrollValues()
    translate(clamp(LOWX, WIDTH/2-center.x, 0), clamp(LOWY, HEIGHT/2-center.y, 0))
    scale(1)
end

Perhaps. Perhaps it should be there but commented out, for efficiency? It might be doing a matrix multiplication behind the scenes. No, we have no justification for that. I’m leaving it, preferring the similarity to the other method.

Commit: refactoring scaling logic.

Pushing too far …

It may be pushing too far, but let’s at least observe that these two methods are now very similar:

function GameRunner:scaleForLocalMap()
    local center = self.player:graphicCorner()
    local LOWX,LOWY = self:maxScrollValues()
    translate(clamp(LOWX, WIDTH/2-center.x, 0), clamp(LOWY, HEIGHT/2-center.y, 0))
    scale(1)
end

function GameRunner:scaleForTinyMap()
    local sc = 0.05
    local dw,dh = Runner:dungeonSize()
    translate(WIDTH-sc*dw, HEIGHT-sc*dh)
    scale(sc)
end

I’d wager that we could remove the duplication there and come down to a single translate-and-scale method that these two call. But since the one is centering on WIDTH/2, HEIGHT/2 and the other is not, I don’t think it’s worth the effort. These are small, and nearly clear as to what they do.

The clamp logic is a bit obscure. I don’t see a good way to describe what that’s doing, but what it’s doing is ensuring that we never scroll so far left, right, up, or down as to display beyond the edge of the map.

Well, it’s 1130, and I started around 0830, so let’s wrap this up and get it out on the web.

Summary

We talked a bit about some of the reasons why I write these things, pointing out that, among other reasons, focusing on telling the story as I go enables me to see and learn from details that would become fuzzy a week, a day, or even an hour later.

Then we looked at some of the design criteria that I’ve learnd to think about from my betters.

Then we looked at some places in my code where those criteria were not well met, and we made that code better. The changes were small and simple, and I find that gratifying, because it suggests to me that the design I have is not too far away from the ideal design I’d have if I were actually good at this stuff.

And it comes about, not thoughtlessly, but mindfully, from the creation of small chunks of working code, over and over, trying to make each chunk as cohesive as I can manage with my limited time and skills.

When I do that, things go better. And when things don’t go so well, we can usually see where I let one or more of those principles and values slip a bit.

Would the same happen for you? I know how I’d bet. But it’s up to you.

See you next year!

D2.zip

  1. Slowly I turned … ↩