Codea Stats 10: Functions
I’m just not up for going back into the Dung mines. Gonna put plain function detection into the stats instead. Also, random subtweeting. Also bear bites man.
Subtweet
This morning I noticed this tweet from GeePaw Hill.
In my daily meandering amongst the stream of geekery tweets, I keep noticing proposed optimizations over MMMSS that seem like they aren’t. I want to write about this, but I haven’t even worked through MMMSS fully yet. It’s very aggra-ma-vatin’.
I’ll remind you that MMMSS is “If you want more value faster, take Many More Much Smaller Steps”, Hill’s1 “rice and garlic2” advice, provided out of context as advice to help you go better. A step is a period of code time where the system isn’t read, between two points where it is ready. It’s ready, we work a bit, during which it’s not ready, then it’s ready again.
Suppose there’s something we can do in two small steps. Is it ever “better” to do it in one larger step?
Sure. Suppose that our definition of “ready” includes “runs all the tests”, which it probably should include. Suppose our tests run for 45 minutes. Then if we do it in two steps, it’ll take at least 90 minutes. If we do it in one step–and get it right–it’ll only take 45 minutes.
Of course there’s a chance of getting the step wrong (at great cost in time, so great that we’re tempted not to run the tests. That’s worth coming back to.) Suppose the chance of making a mistake in either small step, so that the tests don’t run, is one out of four times.
What is the chance of the larger step containing a mistake?
You may be thinking “1/2”, but no. A priori we have a 56% chance of a mistake in the larger step. In reality, our chances are probably worse, because the larger step is more complex than the two smaller ones.
So with the chance of a mistake in a step at 1/4, combining steps seems not to pay off. But if our chance of a mistake were only one in ten, it might pay off.
Yes, but those tests at 45 minutes were the driver. We didn’t even count the time for doing the steps at all.
And if we do make a mistake, what happens? Debugging, figuring out, more testing, more temptation to skip testing …
But those tests are too slow. The tests for my Dung program run in under 0.1 seconds. Getting feedback costs me almost no time at all.
So, there’s no downside to me in taking smaller tests. And yet, if you’ve been watching me, you know that sometimes I take a step that’s too big–and sometimes I even know that the step may be risky.
What’s up with that?
We’re all human. We choose what we do. The trick is to observe what happens and learn from it.
There is no doubt in my mind, based on decades of doing this kind of work, that smaller steps are better. Sometimes I make other choices. Sometimes I get away with it. Sometimes I don’t.
My programming situation is simple in comparison to most of yours. I am less likely to make mistakes that delay me than you are, not that I’m smarter, but that my situation is simpler.
There’s something to think about there.
Functions
Oh, right, we want to do functions.
In Lua, a function definition usually looks like a method definition except without a class name:
function foobar(mumble,baz)
There is another kind of function definition to think about. We could write:
foobar = function(mumble,baz)
We can even write anonymous functions that are never given a name at all, as we do in our CodeaUnit tests:
_:test("HOOKUP", function()
_:expect("Bar", "testing fooness").is("Bar")
end)
I’m not sure whether we should report that third kind, though clearly we could. I do think we should at least do the first two. We’ll start with the first and see what we learn.
Approach
I’ll do my best to TDD this in very small steps, having just given that sermon. And I am supposing we’ll follow the pattern of having a FunctionDescription and FunctionCollection, as we have for Class and Method.
We have some tests that run against our sample program, and some with input provided in the test itself. Here’s one of each:
_:test("Find a class", function()
local pattern = pd:classDefinition()
local start,stop,capture = tab:find(pattern)
_:expect(capture).is("SampleCode")
end)
_:test("Find a subclass", function()
tab = " SubclassOfSample = class ( Sample )"
local pattern = pd:classDefinition()
local start,stop,capture,super = tab:find(pattern)
_:expect(capture).is("SubclassOfSample")
_:expect(super).is("Sample")
end)
I’ll do some “find a function” ones.
_:test("Find a named function", function()
tab = "\n function foobar ( baz, mumble )\n"
local pattern = ""
local start, stop, functionName, args = tab:find(pattern)
_:expect(functionName).is("foobar")
_:expect(args).is("baz, mumble")
end)
That should be enough to fail.
4: Find a named function -- Actual: nil, Expected: foobar
I needed to turn off the display of all the projects in the universe, and the touch sensitivity. If these keeps up I might add a toggle switch to turn on real operation.
Anyway, we need a pattern.
The method definition pattern is this:
"%s*function%s*(%w+)%s*:%s*(%w+)%s*%(%s*(.-)%s*%)"
We can trim that down to get our function one.
_:test("Find a named function", function()
tab = "\n function foobar ( baz, mumble )\n"
local pattern = "%s*function%s*%s*(%w+)%s*%(%s*(.-)%s*%)"
local start, stop, functionName, args = tab:find(pattern)
_:expect(functionName).is("foobar")
_:expect(args).is("baz, mumble")
end)
This step is over. The tests are green. Commit: can recognize simple function with args.
We’ll move the pattern to the dictionary.
_:test("Find a named function", function()
tab = "\n function foobar ( baz, mumble )\n"
local pattern = pd:namedFunctionDefinition()
local start, stop, functionName, args = tab:find(pattern)
_:expect(functionName).is("foobar")
_:expect(args).is("baz, mumble")
end)
function PatternDictionary:namedFunctionDefinition()
return "%s*function%s*%s*(%w+)%s*%(%s*(.-)%s*%)"
end
This step is over. Tests green. Commit: namedFunctionPattern added to dictionary.
We could go two different ways at this point. We could add other forms of function definition, or we could move to get this form into the report.
I think the latter idea has more value. Let’s try to think of a few ways to go there.
We’ll wind up adding some function definitions to the sample program, and, I think, adding them to the full report. (I have in mind displaying naked functions before classes.)
We do the report generation using MatchMaker instances. Those instances scan tabs and produce collections of various kinds:
An example test is:
_:test("MatchMaker finds methods", function()
local expected = [[ SampleCode:init(x)
SampleCode:draw()
SampleCode:touched(touch)
SubclassOfSample:init(ignored)
SampleCode:extraMethod(a,b,c)
]]
local mm = MatchMaker:methodDefinition()
local tab = readProjectTab("SampleStatsProject:SampleCode")
tab = stripComments(tab)
mm:process(tab)
local coll = mm:collection()
local isClass = coll:is_a(MethodCollection)
_:expect(isClass, "method collection").is(true)
_:expect(coll:count()).is(5)
local rep = coll:report()
_:expect(rep).is(expected)
end)
So one way is to do the MatchMaker test for named functions. That test above uses the sample project but we could build our own if we wanted to.
What’s another way? We could start with the ProjectDescription tests:
_:test("ProjectDescription classes", function()
local desc = ProjectDescription("SampleStatsProject")
local classes = desc:classes()
_:expect(classes:count()).is(2)
end)
We’d do a similar simple test. It looks like the natural flow for either of these options, is to put some functions into the sample program, and work from there. We should ensure that adding those sample lines doesn’t break anything else.
I don’t see another starting point offhand. This should be easy enough. Add lines to the Sample. I’ll add some that I do not expect to find as well as some that I do expect to find.
I add these lines:
function find3Args ( arg1, arg2, arg3 )
end
find2Args = function (number1, number2) end
Now a test. I think I’ll start with the ProjectDescription and work downward:
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local methods = desc:namedFunctions()
_:expect(methods:count()).is(1)
end)
Our step begins. Test.
2: ProjectDescription named functions -- TabTests:22: attempt to call a nil value (method 'namedFunctions')
Implement:
function ProjectDescription:namedFunctions()
end
Now nothing that knows count.
2: ProjectDescription named functions -- TabTests:23: attempt to index a nil value (local 'methods')
That message confused me. I didn’t rename the local when I copied the test. This is more like it:
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
end)
2: ProjectDescription named functions -- TabTests:23: attempt to index a nil value (local 'functions')
We need to return something. Now if I’m in a hurry to get this test run, I could return a methodCollection or something like that, that does understand count. I could even build a fake object.
I’m trying to take small steps, and this one is a bit larger than I’d like, but I know it’s only this far from working:
function ProjectDescription:namedFunctions()
return FunctionCollection()
end
FunctionCollection = class()
function FunctionCollection:count()
return 1
end
The test passes. Enhance the test. We’ll ask our collection for its report:
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
local rep = functions:fullReport()
_:expect(rep).is("find3args(arg1, arg2, arg3)")
end)
Test should fail with no such method.
2: ProjectDescription named functions -- TabTests:24: attempt to call a nil value (method 'fullReport')
Implement it. Let’s make it incorrect.
function FunctionCollection:fullReport()
return "bad answer"
end
I expect that to show up in the error.
2: ProjectDescription named functions -- Actual: bad answer, Expected: find3args(arg1, arg2, arg3)
The FunctionCollection is supposed to contain FunctionDefinitions. Let’s drive that out.
You know what? This path isn’t as good as I thought, because I really need to work through the whole MatchMaker thing.
I see two possibilities. Revert, or gut it through. Well, a third possibility is to ignore this test and drive out MatchMaker. Let’s do that.
We must talk about this in terms of MMMSS soon.
MatchMaker
Here’s a test, emulating the other matchmaker tests. (Why do I type mathmaker or worse every time?)
_:test("MatchMaker finds named functions", function()
local mm = MatchMaker:namedFunctionDefintion()
local tab = readProjectTab("SampleStatsProject:SampleCode")
tab = stripComments(tab)
mm:process(tab)
local coll = mm:collection()
local isFunc = coll:is_a(FunctionCollection)
end)
Test should tell us we need namedFunctionDefinition.
3: MatchMaker finds named functions -- MatchMaker:50: attempt to call a nil value (method 'namedFunctionDefintion')
function MatchMaker:namedFunctionDefinition()
return MatchMaker(FunctionCollection, FunctionDefinition, PatternDictionary:namedFunctionDefintion())
end
That’s just the standard way to define that. What breaks? I don’t know:
3: MatchMaker finds named functions -- MatchMaker:50: attempt to call a nil value (method 'namedFunctionDefintion')
Spelling error. Aren’t you glad the tests don’t run 45 minutes?
I had the same mistake in two places. There’s 90 minutes burned. Now we get an error we can work with:
3: MatchMaker finds named functions -- MatchMaker:84: attempt to call a nil value (field 'defn')
Right. We don’t even have a FunctionDefinition class yet.
FunctionDefinition = class()
function FunctionDefinition:init(functionName, args)
self.functionName = functionName
self.args = args
end
I wrote more code than I need. I must be getting panicky.
3: MatchMaker finds named functions -- MatchMaker:85: attempt to call a nil value (method 'add')
No add method yet on FunctionCollection.
function FunctionCollection:add(anItem)
table.insert(self.functionDefinitions, anItem)
end
Test. We’re back, as expected to this:
2: ProjectDescription named functions -- Actual: bad answer, Expected: find3args(arg1, arg2, arg3)
We need a better report:
I start with this:
function FunctionCollection:fullReport()
local result = ""
return result
end
Why? Because I often forget the return of an accumulating value, so I’m trying to get in the habit of typing them in right away.
But the code:
function FunctionCollection:fullReport()
local result = ""
for _i,func in ipairs(self.functionDefinitions) do
result = result..func:fullReport()
end
return result
end
Test.
2: ProjectDescription named functions -- Actual: , Expected: find3args(arg1, arg2, arg3)
Hm an empty string. What is my fullReport on functionDefinition? Do I even have one? I do not. How did this not error?
I’m out on the ledge here. There’s nothing for it but to push forward … or, of course, to revert and begin again. That last commit was an hour ago! I can’t afford to lose an hour’s work, can I?
Well, not yet. Let’s see what’s going on here.
This test seems happy:
_:test("MatchMaker finds named functions", function()
local mm = MatchMaker:namedFunctionDefinition()
local tab = readProjectTab("SampleStatsProject:SampleCode")
tab = stripComments(tab)
mm:process(tab)
local coll = mm:collection()
local isFunc = coll:is_a(FunctionCollection)
end)
This one does not:
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
local rep = functions:fullReport()
_:expect(rep).is("find3args(arg1, arg2, arg3)")
end)
Ah. I guess we just didn’t get any reports out, but why didn’t it error?
I change the output:
function FunctionCollection:fullReport()
local result = "Functions\n\n"
for _i,func in ipairs(self.functionDefinitions) do
result = result..func:fullReport()
end
return result
end
This should fail showing functions. The only way, I think, that this can fail is if we have no definitions. Test.
2: ProjectDescription named functions -- Actual: Functions
, Expected: find3args(arg1, arg2, arg3)
OK, that tells us we’re getting in there. But it must be that we have no definitions or we’d be calling fullReport on them, which would fail. That’s what I expected.
So the match didn’t work. Do we even have one?
_:test("MatchMaker finds named functions", function()
local mm = MatchMaker:namedFunctionDefinition()
local tab = readProjectTab("SampleStatsProject:SampleCode")
tab = stripComments(tab)
mm:process(tab)
local coll = mm:collection()
local isFunc = coll:is_a(FunctionCollection)
_:expect(coll:count()).is(1)
_:expect(coll:fullReport()).is("no idea")
end)
The count passes, I already know. Test.
3: MatchMaker finds named functions -- TabTests:156: attempt to call a nil value (method 'fullReport')
But wait. I thought we knew it was a FunctionCollection. Don’t we have fullReport? Yes but not on FunctionDefinition. I can work with this but am not sure why the original test isn’t working differently. I should have truly ignored it.
function FunctionDefinition:fullReport()
return self.functionName.."("..self.args..")\n"
end
I realize, again, that my management of who says newline and who inserts tabs is weird. But this should get me a more credible test result.
3: MatchMaker finds named functions -- Actual: Functions
find3Args(arg1, arg2, arg3)
, Expected: no idea
That’s what I’m talkin about!
Now let’s remove the header, and change the test to expect, well, what’s happening, because by inspection it’s right.
_:test("MatchMaker finds named functions", function()
local mm = MatchMaker:namedFunctionDefinition()
local tab = readProjectTab("SampleStatsProject:SampleCode")
tab = stripComments(tab)
mm:process(tab)
local coll = mm:collection()
local isFunc = coll:is_a(FunctionCollection)
_:expect(coll:count()).is(1)
_:expect(coll:fullReport()).is("find3Args(arg1, arg2, arg3)\n")
end)
And …
3: MatchMaker finds named functions -- OK
The matchmaker test is good and it’s actually working as intended. What about the other one?
2: ProjectDescription named functions -- Actual: , Expected: find3args(arg1, arg2, arg3)
Here’s that test:
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
local rep = functions:fullReport()
_:expect(rep).is("find3args(arg1, arg2, arg3)")
end)
I could ignore it literally and commit now. In fact let’s do.
Commit: matchMaker makes named function definitions correctly.
OK, now we’re back off the ledge. That took just short of 90 minutes. Way too big a step. Starting with MatchMaker would have been better.
Working bottom up from FunctionDefinition and FunctionCollection would have been smaller, but the risks there are a bit higher in that they might not match up with the higher-level needs. In this case, that’s not likely, because they are just like the other classes for Class and Method.
I really wish I had taken MMMSS, but at least I’ve mostly survived.
Now what’s up with that last test?
function ProjectDescription:namedFunctions()
return FunctionCollection()
end
That could have something to do with it. I stubbed that and never got back to it.
Here’s the init:
function ProjectDescription:init(projectName)
self.projectName = projectName
self.tabs = self:getProjectTabs(projectName)
local classMatcher = MatchMaker:classDefinition()
local methodMatcher = MatchMaker:methodDefinition()
for _i,tab in ipairs(self.tabs) do
local cont = tab:contents()
cont = stripComments(cont)
classMatcher:process(cont)
methodMatcher:process(cont)
end
self.classCollection = classMatcher:collection()
self.methodCollection = methodMatcher:collection()
end
Clearly we need a function matcher and to run it.
function ProjectDescription:init(projectName)
self.projectName = projectName
self.tabs = self:getProjectTabs(projectName)
local classMatcher = MatchMaker:classDefinition()
local methodMatcher = MatchMaker:methodDefinition()
local namedFunctionMatcher = MatchMaker:namedFunctionDefinition()
for _i,tab in ipairs(self.tabs) do
local cont = tab:contents()
cont = stripComments(cont)
classMatcher:process(cont)
methodMatcher:process(cont)
namedFunctionMatcher:process(cont)
end
self.classCollection = classMatcher:collection()
self.methodCollection = methodMatcher:collection()
self.namedFunctionCollection = namedFunctionMatcher:collection()
end
But we need to do better on the namedFunctions method as well.
function ProjectDescription:namedFunctions()
return self.namedFunctionsCollection
end
Now let’s test.
2: ProjectDescription named functions -- TabTests:23: attempt to index a nil value (local 'functions')
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
local rep = functions:fullReport()
_:expect(rep).is("find3args(arg1, arg2, arg3)")
end)
Hm we returned a nil from namedFunctions??
function ProjectDescription:namedFunctions()
return self.namedFunctionsCollection
end
Spelling error.
function ProjectDescription:namedFunctions()
return self.namedFunctionCollection
end
Test and we get a surprise!
2: ProjectDescription named functions -- Actual: setup()
draw()
find3Args(arg1, arg2, arg3)
, Expected: find3args(arg1, arg2, arg3)
What’s up? The ProjectDescription test reads all the tabs of the project not just the sample. There are no classes and methods in the Main tab, but there is a setup and draw:
-- SampleStatsProject
function setup()
end
function draw()
end
Neat Fix the test:
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
local rep = functions:fullReport()
_:expect(rep).is("setup()\ndraw()\nfind3args(arg1, arg2, arg3)\n")
end)
I’m pretty sure there should be newlines throughout like that. However, the test does not agree:
2: ProjectDescription named functions -- Actual: setup()
draw()
find3Args(arg1, arg2, arg3)
, Expected: setup()
draw()
find3args(arg1, arg2, arg3)
Ah. Didn’t capitalize the “a”.
_:test("ProjectDescription named functions", function()
local desc = ProjectDescription("SampleStatsProject")
local functions = desc:namedFunctions()
_:expect(functions:count()).is(1)
local rep = functions:fullReport()
_:expect(rep).is("setup()\ndraw()\nfind3Args(arg1, arg2, arg3)\n")
end)
This better run.
2: ProjectDescription named functions -- OK
At last. Commit: ProjectDescription finds named functions.
I’m of a mind to stop but we aren’t putting the named functions into the full report yet.
Let’s at least do a Retro and then decide.
Retrospective
So did you see that? All that sermonizing about small steps, a legitimate attempt to choose small steps, and still I got into an hour or so of trouble.
I had honestly hoped for much better. I was hoping we’d go step commit step commit step commit and be all good. I guess it’s a case of some of us mostly serving as bad examples for the rest of us.
But seriously, I don’t feel badly about what happened: it is quite typical of what happens when we (or at least when I) program. I do, however, feel tense and nervous, because I was out on that ledge a lot longer than I ever want to be. My nominal upper limit for time between tests is maybe 20 minutes and this was over four times that.
What do we learn? Well, “do better” isn’t very good advice. I do have a few ideas:
- More Bottom Up
- If I had started with the function definition and collection, I might have at least had those in place with small steps. Then probably the new capability on MatchMaker would have been easy. Then, finally, maybe ProjectDescription. So I think I kind of went in the wrong order.
-
I had a decent reason: I wanted to go top down to be sure that my underlying objects were serving me well. But a quick count of the classes and methods it would take to make it good could have tipped me off.
- Objects Not Right?
- I think maybe these objects aren’t right. They are quite similar, with all the same methods and no real differences other than in how many things the pattern matches (which is generalized anyway) and in the actual form of the output line. It seems likely that that batch of things could be collapsed, and if it were, today might have gone more smoothly.
-
That said, we only had two cases when we started, Method and Class, and this is the third. I could make a case that it would be premature to move to remove the duplication with only two cases. Looking at it now, however, the pattern has held up and we should probably collapse things if we can.
- String Testing
- The report testing comes down to matching strings, and I’ve made a few mistakes with typos in the strings, missing newlines, and the like. This argues for better diagnosis of mismatching strings, but I don’t think that did us much harm today.
Overall, I would really like to have taken smaller steps, and I think the biggest reason I didn’t was that I made a poor choice of where to start. A wiser man might have reverted, but ignoring the test didn’t hurt much. It did hurt a little, when I noticed its failure and found it surprising. Staying heads down on the MatchMaker test would have been a bit better.
I had hoped to demonstrate that MMMSS is a good thing by doing it well. I think what I may have demonstrated instead is that there’s not much of a case for larger steps. Even the small ones today were pretty clearly too large.
Changes to make? We’ll try to focus on smaller steps. We’ll make a card for improving CodeaUnit’s string comparisons.
Now … shall we add functions to the report today?
Here’s the code for full report:
function ProjectDescription:fullReport()
report = "Classes\n\n"
report = report..self.classCollection:fullReport(self.methodCollection)
return report
end
Let’s not. Why? Because I think we should push the headings downward. If we put a “Functions” heading in here, we’d need an if statement to decide whether to do it or not.
We’ll save that for a fresh morning.
See you then, I hope!