Today I plan to just look for some small opportunities to improve the code. I will be particularly looking for ways to use my fp primitives, and to improve them to make them more useful.

I haven’t given up on my functional programming operations, but it sure does seem that they aren’t as useful to my style of programming as I had hoped. So I’m going to start my explorations here looking at loops, but I’m open to finding any kinds of improvements.

The target program, D2, informally called Dung[eon], is my largest Codea program by far and the small refactorings we do are not likely to have any major impact. We’re really just practicing small improvements, along the lines of Kent Beck’s new work on Tidy First. Highly recommended, by the way.

I’ll start with one that Laurent Bossavit found:

function Dungeon:nearestContentsQueries(tile)
    for i,tile in ipairs(self:neighbors(tile)) do
        tile:doQueries()
    end
end

What’s up with overriding the name tile that way? It can only be confusing. Let’s fix that.

function Dungeon:nearestContentsQueries(anyTile)
    for i,neighbor in ipairs(self:neighbors(anyTile)) do
        neighbor:doQueries()
    end
end

That’s surely better. And here’s something odd. I looked a the neighbors function, which happens to be next, and the function that it uses:

function Dungeon:neighbors(tile)
    local tPos = tile:pos()
    local offsets = self:neighborOffsets()
    return map(offsets, function(offset) return self:getTile(offset + tPos) end)
end

function Dungeon:neighborOffsets()
    local dirs = Maps:allNeighborDirections()
    local function toOff(dir)
        return dir:toCartesianOffset()
    end
    local offsets = map(dirs, toOff)
    return offsets
end

Would you believe that there is already a map function in the Dung program? I had forgotten that arguably key fact. Let me look for it. Well, look at that, in a tab called TableOperations.

function map(tab,f)
    local r = {}
    for k,v in ipairs(tab) do
        r[k] = f(v)
    end
    return r
end

I’ve also got arraySelect and tableSelect in there, and an off function asTable.

function asTable(something)
    if type(something) == "table" then
        return something
    else
        return {something}
    end
end

Well. This would be embarrassing if I had any shame at all about my programming, but I’ve made so many mistakes programming that now I just offer them up to my readers as examples that we make mistakes in this business, and that a key aspect of being any good at it at all is to deal with that truth in effective ways.

Back in Dungeon 124 I implemented a class called Collection, and then the methods mentioned above, and then removed Collection as unneeded.

Are these things used? We see two uses of map.

I find one use of asTable:

function FSM:fromMatches(trans)
    for i,fr in ipairs(asTable(trans.from)) do
        if fr == self.fsm_state then return true end
    end
    return false
end

That’s used exactly once:

function FSM:acceptedTransition(event)
    for i,trans in ipairs(self.tab.events) do
        if self:fromMatches(trans) and trans.event == event then
            return trans
        end
    end
    return nil
end

What’s up with the trans.from that requires us to force it to a table? Apparently it was done for “convenience”:

                events = {
                    {event="step",from="m1",to="m2"},
                    {event="step",from="m2",to="m3"},
                    {event="jump",from={"m1","m2","m3"},to="done"},
                    {event="back",from="done",to="m1"},
                }

In setting up a FiniteStateMachine, I allow myself to list a single from or a collection. So the asTable call allows me to forget typing {} around a single transition. Of course I’d have to review how FSM is used, but today this seems to me to be a nicety that I could do without. It is a nicety, however.

It turns out that in addition to the two above, there is one more use of map:

function Decor:createRequiredItemsInEmptyTiles(items, runner)
    return map(items, function(item)
        local tile = runner:randomRoomTile(666)
        return Decor(tile,item)
    end)
end

In for a penny, what about the arraySelect and tableSelect?

function Dungeon:availableTilesAtDistanceFromTile(startingTile, targetTile, desiredDistance)
    local atCorrectDistance = function(tile)
        return targetTile:distanceFrom(tile) == desiredDistance
    end
    local tileIsFloor = function(tile)
        return tile:isFloor()
    end
    local neighbors = startingTile:reachableTilesInMap(self.map)
    local correctDistance = arraySelect(neighbors, atCorrectDistance)
    local floorTiles = arraySelect(correctDistance, tileIsFloor)
    return #floorTiles > 0 and floorTiles or { startingTile }
end

There are no uses of tableSelect.

Now it seems to me that I should be able to replace these functions with references to my new ones. I think I’ll try it.

function Dungeon:availableTilesAtDistanceFromTile(startingTile, targetTile, desiredDistance)
    local atCorrectDistance = function(tile)
        return targetTile:distanceFrom(tile) == desiredDistance
    end
    local tileIsFloor = function(tile)
        return tile:isFloor()
    end
    local neighbors = startingTile:reachableTilesInMap(self.map)
    local correctDistance = ar.filter(neighbors, atCorrectDistance)
    local floorTiles = ar.filter(correctDistance, tileIsFloor)
    return #floorTiles > 0 and floorTiles or { startingTile }
end

This works. I’ll try map.

function Dungeon:neighbors(tile)
    local tPos = tile:pos()
    local offsets = self:neighborOffsets()
    return ar.map(offsets, function(offset) return self:getTile(offset + tPos) end)
end

function Dungeon:neighborOffsets()
    local dirs = Maps:allNeighborDirections()
    local function toOff(dir)
        return dir:toCartesianOffset()
    end
    local offsets = ar.map(dirs, toOff)
    return offsets
end

Those work as well. I am not surprised, or at least not very surprised. There’s always a bit of concern but my tests pass and the game works.

Now for asTable. We recall that it’s used just this once:

function FSM:fromMatches(trans)
    for i,fr in ipairs(asTable(trans.from)) do
        if fr == self.fsm_state then return true end
    end
    return false
end

Let’s write the code out longhand and then see what we have here.

function FSM:fromMatches(trans)
    if type(trans.from) ~= "table" then trans.from = {trans.from} end
    for i,fr in ipairs(trans.from) do
        if fr == self.fsm_state then return true end
    end
    return false
end

That’s a bit nasty because it modifies the input table trans. A bit rude. But wait:

function FSM:fromMatches(trans)
    local from = trans.from
    if type(from) ~= "table" then return from==self.fsm_state end
    for i,fr in ipairs(from) do
        if fr == self.fsm_state then return true end
    end
    return false
end

We could do that, checking the single one explicitly. Also we can do this:

function FSM:fromMatches(trans)
    local from = trans.from
    if type(from) ~= "table" then return from==self.fsm_state end
    return ar.exists(from,function(fr) return fr == self.fsm_state end)
end

This works but I think we can do better.


function FSM:fromMatches(trans)
    local match = function(fr) return fr==self.fsm_state end
    local from = trans.from
    if type(from) ~= "table" then 
        return match(from)
    else
        return ar.exists(from, match)
    end
end

Now I really prefer forcing the from to table. Let’s try it:

function FSM:ensureTable(instanceOrTable)
    if type(instanceOrTable) == "table" then
        return instanceOrTable
    else
        return { instanceOrTable }
    end
end

function FSM:fromMatches(trans)
    local match = function(fr) return fr==self.fsm_state end
    local from = self:ensureTable(trans.from)
    return ar.exists(from, match)
end

We have to pay a price for allowing users to put single instances or tables of instances into FSM tables. Now we pay it right here, inside FSM.

I think we can now remove the entire TableOperations tab.

Let’s commit: remove all references to TableOperations, replacing with functions from fp library.

Now let’s remove the TableOperations tab and test again.

Moohaha, found another one, in Decor. Revert.

function Decor:createRequiredItemsInEmptyTiles(items, runner)
    return map(items, function(item)
        local tile = runner:randomRoomTile(666)
        return Decor(tile,item)
    end)
end

Replace with ar.map, test. Tests run. Commit: use ar.map in Decor. Delete TableOperations tab again. Test. We’re good.

Commit: Remove TableOperations in favor of fp library.

OK, so that’s nice. Let’s do an interim retrospective.

Retro

Triggered by an observation from Laurent, we observed some very confusing naming, and in fixing that, discovered entirely by accident that there was already a rudimentary map operation built into D2, and array and table selections as well.

Keen to use the new stuff, I (taking all the blame here) rooted around and replaced all the calls to map and arraySelect. Along the way, tests saved me (still taking the blame) from a number of mistakes. In the end it works and we (you get some of the credit) now have the dubious advantage of using the official functional programming library of our organization “fp”.

We’re left with the oddity ensureTable, which is only provided to allow finite state machine tables to be created with either a single from or a table of them. Do we use that capability? Interesting question. In fact we do. And, mind you, an FSM table is a pretty big deal. Here’s the only one we have, for the Horn Girl, who has lost her Amulet of Cat Persuasion and hopes the Princess can find it for her:

function NPC:hornGirlTable()
    local tbl = {
        initial="unsat",
        events = {
            {event="query", from="unsat", to="wait", arg="unsat"},
            {event="query", from="wait", to="wait",arg="wait"},
            {event="received", from={"unsat","wait"}, to="thanks", arg="thanks"},
            {event="query", from="thanks", to="satisfied", arg="satisfied"},
            {event="query", from="satisfied", to="satisfied", arg="satisfied"},
        },
        callbacks = {
            on_leave_unsat = function(fsm,event,from,to, arg)
                if event ~= "query" then return end
                return self:publishTale(fsm, arg)
            end,
            on_leave_wait = function(fsm,event,from,to, arg)
                if event ~= "query" then return true end
                return self:publishTale(fsm, arg)
            end,
            on_enter_thanks = function(fsm,event,from,to, arg)
                return self:publishTale(fsm, arg)
                -- give something
            end,
            on_enter_satisfied = function(fsm,event,from,to, arg)
                return self:publishTale(fsm, arg)
            end,
        },
        data = {
            unsat = Tale{
                "I am your friendly neighborhood Horn Girl!\nAnd do I have a super quest for you!",
                "I am looking for my lost Amulet of Cat Persuasion,\nso that I can persuade the Cat Girl to let me pet her kitty.",
                "If you can find it and bring it to me,\nI shall repay you generously.",
            },
            wait = Tale{
                "I sure hope you can find the Amulet for me!\nI'll be so grateful!",
            },
            thanks = Tale{
                "Thank you so much!\nPlease accept this useful gift!",
            },
            satisfied = Tale{
                "Hello, friend!",
            },
        }
    }
    return tbl
end

For the terminally masochistic among us, the writeup of the FSM starts in Dungeon: FSM, although there is a manual implementation of Horn Girl starting around Dungeon 210.

It occurs to me that it might be interesting to come up with some more NPCs, and use that need to drive out more of a “Making App” for the dungeon program, allowing a non-programming developer to create interesting encounters and such. Might be interesting enough that I’ll actually do it. I can’t wait to find out.

Summary

I’d like to stop here. I’m relatively happy, given the state of the world, because my fp experiment has turned out to be somewhat useful in the D2 program, and has resulted in some effects that are worth nothing, because they’re not unique to this specific situation.

Thing one, we were able to remove a tab of code from our app, consisting of tests and code for a limited set of functional programming primitives that existed in the global name space. We replaced them with a library referenced in as a dependency, which does NOT contaminate the global name space so badly, and which offers (we can imagine) better curated more general capabilities than our very specific ones. And plugging them in was pretty easy. We could have done each of those changes separately and evolved away from TableOperations over many releases in a real situation.

We started, not with intention to accomplish that arguably desirable goal, but to improve a small infelicity in the code, which used the term tile with two different meanings within a single line. (A possible contender for some kind of ignoble award.) But once we started to do that bit of tidying, we explored further and found the opportunity to move to our new library, and to improve some other code using our exists function, which was not even available to us prior to the fp library.

We have to ask ourselves whether this kind of tidying is “worth it”. If we were under incredible pressure to add features to the dungeon, maybe it wouldn’t be worth it. But I suggest that it would be.

Why? Because while under pressure, we wouldn’t likely have found this specific problem, we would very likely find some similar problem in whatever code we were actually working on. Why? Because we tend to program the same way day to day and to leave the same bits of untidy stuff lying about. So when we encounter those things in code we’re working on, improving that code is likely to pay off, exactly because we’re working on it and infelicities slow us down.

That’s why we follow the camper’s rule of always leaving the campground a bit cleaner than you found it. Over time, it pays off, because when we come back, or our colleague comes back, their job will be a bit easier, and thus faster and more reliable.

For those reasons, I prefer always to improve the code I’m working on, if I can see a way to do it. And there’s one more reason that is important to me.

Back when I worked for a living, I enjoyed the fact that they paid me, so that I was able to support my family and occasionally my love for fun cars. But day in and day out, if the job was not enjoyable, the money wasn’t enough. I was fortunate enough to get into this work in a situation where I encountered the joy of learning and the joy of doing something well (or at least better than I previously had), and I’ve always known that I prosper emotionally when I can be proud of my work.

And when we prosper emotionally, we do better work, and there’s a chance, maybe even a decent chance, that someone will notice and pay us a bit more. But even if they don’t … I’ll take less money and more happiness over less happiness and more money.

YMMV, but I do recommend that folx work in a way that provides them pride and learning.

Today was a day like that, and it’s one that most anyone could emulate if they chose to, improving the work right where they are working today.

See you next time! I have no idea what we’ll do.