Codea Stats 4

Backward? Or forward? Which was it?

Yesterday, GeePaw Hill posted an extended “Muse” on the notion of Many More Much Smaller Steps. I urge you to read it and consider it carefully.

In that Muse, he speaks in terms of choosing steps toward “better”, without defining the term, and I thought it was interesting that yesterday, on this Codea Stats program, I could be said to have taken a step backward. A couple of days back, I had method definitions and class definitions being collected, and with yesterday’s design change, I got method definitions done, but not class definitions. This could be seen as a step toward “worse”, at least as regards features in the program.

I could argue that the program was net “better” because of a better design, though the strength of that new design can hardly be said to have been given much of a trial yet. But I am confident that it’s better than it was. But we did lose a “feature”.

Frankly, that’s not good. I believe that things go best when we make steady progress in program capability, moving toward our changing vision of what the product should be. If a product has been released, we’re not likely to release a new version with fewer capabilities. If we were to do that, it would require some serious high-level decision-making, in my view. Given my standard fears during a software effort, I’d be very unlikely to recommend a reduction in capability, even to get some new good thing.

It’s something to think about. I could, I’m sure, have refactored toward the new design, and left the class definition working, and brought it into the fold soon enough. However, it seemed to me that at the stage the program was in, a quick rewrite was better. And, had breakfast not intervened, the question might not have come up.

I’m glad it did, though. When we work in tiny steps, Hill’s MMMSS, it is as if we are picking our way along a wandering path up a hill, choosing our next position both for its ability to get us a bit higher, but also to enable further steps. I am not a rock climber, but I imagine that one always wants to make moves that are not too far, and that lead to further moves that are also not too far. So one makes a local decision in the presence of an evolving overall plan.

I used to be able to shoot pool, or thought that I could, until James Grenning showed me otherwise. (Do not play against him for big money, that’s my advice.) In pool, you’re always thinking about two issues. First, to pot another ball, and second, to leave yourself in position for further balls. (And, if you can’t pot one, you play to leave you opponent, in bad “shape”.) You can often think two or more shots ahead, but things often change before you get there.

It’s much the same here. We are making small steps along a path that may turn out to be good, but when we get a bit further along, we often see a better path.

And it’s good not to have to go back, but sometimes, well, the bear bites you. Or Grenning makes some impossible shot.

Let’s Get To It

Time to code. Where are we? I have no specific recollection. It has been hours since I last looked at the code.

Ah, I see here in the tests:

        _:test("ProjectDescription methods", function()
            local desc = ProjectDescription("SampleStatsProject")
            local methods = desc:methods()
            _:expect(methods:count()).is(5)
            local sampleCodeMethods = desc:methods("SampleCode")
            _:expect(sampleCodeMethods:count()).is(4)
        end)

We have the new object ProjectDescription, which is going to hold all the info we analyze out of the program we’re, well, analyzing. And right now, when it’s created, it has a collection of methods, and it can return all the methods, or the methods for a given class. I remember thinking that we may want a more robust query approach.

We don’t have any checks on what is actually in the MethodsCollection. The code is doing more than we’ve tested:

function ProjectDescription:addMethodsFromTab(tab, methods)
    local tabContents = tab:contents()
    local pattern = PatternDictionary:methodDefinition()
    local matchResult = tabContents:gmatch(pattern)
    for className,methodName,args in matchResult do
        local method = MethodDefinition(className,methodName,args)
        table.insert(methods, method)
    end
end

MethodDefinition = class()

function MethodDefinition:init(className, methodName, args)
    self.className = className
    self.methodName = methodName
    self.args = args
end

We have no test that requires us to save the method name and args. We do arguably have one for the class name, because this:

function MethodCollection:methodsForClass(className)
    local result = {}
    for _i,meth in ipairs(self.methodDefinitions) do
        if not className or meth.className == className then
            table.insert(result, meth)
        end
    end
    return MethodCollection(result)
end

Speaking of that, the code here is ripping the guts out of our MethodDefinition. Let’s fix that up a bit.

function MethodCollection:methodsForClass(className)
    local result = {}
    for _i,meth in ipairs(self.methodDefinitions) do
        if not className or meth:inClass(className) then
            table.insert(result, meth)
        end
    end
    return MethodCollection(result)
end

We’ll also need:

function MethodDefinition:inClass(className)
    return self.className == className
end

I expect the tests to run. They do. Commit: push inClass check down to MethodDefinition.

We could write tests to require the method name to be there and such, and perhaps we should. Let me know in your cards and letters and tweets. What we’re going to do, however, is to collect all the class definitions,

        _:test("ProjectDescription classes", function()
            local desc = ProjectDescription("SampleStatsProject")
            local classes = desc:classes()
            _:expect(classes:count()).is(2)
        end)

That should be enough to cause us to do the work. We could, in principle, use “fake it till you make it” to drive out the method in trivial form, but I’m feeling strong, and I know you like to watch me crash and burn, so I’m going for it.

I write this by cribbing from getMethods:

function ProjectDescription:init(projectName)
    self.projectName = projectName
    self.tabs = self:getProjectTabs(projectName)
    self.methodCollection = self:getMethods()
    self.classCollection = self:getClasses()
end

function ProjectDescription:getClasses()
    local classes = {}
    for _i,tab in ipairs(self.tabs) do
        self:addClassesFromTab(tab,classes)
    end
    return ClassCollection(classes)
end

Now for the addClasses and ClassCollection:

function ProjectDescription:addClassesFromTab(tab, classes)
    local tabContents = tab:contents()
    local pattern = PatternDictionary:classDefinition()
    local matchResult = tabContents:gmatch(pattern)
    for className, superClassName in matchResult do
        local def = ClassDefinition(className,superClassName)
        table.insert(classes)
    end
end

Oh, and ClassDefinition. This is getting to be a lot of code. It’s only a few minutes work but it’s more than I feel I should be doing. Still, we’re in the deep water now, gotta keep swimming, I guess.

ClassDefinition = class()

function ClassDefinition:init(className,superclassName)
    self.className = className
    self.superclassName = superclassName
end

Easy enough. Again more code than is required.

ClassCollection = class()

function ClassCollection:init(classTable)
    self.classDefinitions = classTable
end

function ClassCollection:count()
    return #self.classDefinitions
end

This should be nearly good. Test.

1: ProjectDescription methods -- TabTests:73: wrong number of arguments to 'insert'

function ProjectDescription:addClassesFromTab(tab, classes)
    local tabContents = tab:contents()
    local pattern = PatternDictionary:classDefinition()
    local matchResult = tabContents:gmatch(pattern)
    for className, superClassName in matchResult do
        local def = ClassDefinition(className,superClassName)
        table.insert(classes)
    end
end

I had a typo in the insert, and in fixing it, broke it. Should be:

        table.insert(classes, def)

Test.

2: ProjectDescription classes -- TabTests:26: attempt to call a nil value (method 'classes')

Better implement that.

function ProjectDescription:classes()
    return self.classCollection
end

Test. Test runs. I’m not satisfied with the tests but I am confident that it’s working. We’ll come back to that weird notion. First, commit: ProjectDefinition has classes returning class definitions.

What Are You Talkin’ ‘bout, Ron?

We have values saved in the ClassDefinition and MethodDefinition which are not tested. The methodName, and args in MethodDefinition and className in ClassDefinition could be set to argle-bargle and the tests would all run.

Let’s improve the tests a bit.

I’d prefer to improve them in aid of the actual application, not just randomly. The application will produce a report, a string containing some nicely formatted output describing the situation.

Let’s go for it. We’ll test the report: that will certainly test the field values.

        _:test("ProjectDescription classReport", function()
            local desiredReport = [[Classes

SampleCode
SubclassOfSample
]]            
            local report = ProjectDescription("SampleStatsProject"):classReport()
            _:expect(report).is(desiredReport)
        end)

I expect to enhance this test with formatting, but it’s more than enough for now.

function ProjectDescription:classReport()
    report = "Classes\n\n"
    report = report..self.classCollection:report()
    return report
end

I was wondering how to unwind the class collection from ProjectDescription when I remembered this amazing thing called “delegation”, so I’m asking the classCollection to do it.

function ClassCollection:report()
    result = ""
    for _i,classDef in ipairs(self.classDefinitions) do
        result = result..classDef:report().."\n"
    end
    return result
end

I’m starting to like this delegation idea.

function ClassDefinition:report()
    return self.className
end

Let’s see what our test says. What is says is OK. I gotta break it just to see it fail. Let’s do some formatting:

            local desiredReport = [[Classes

    SampleCode
    SubclassOfSample(SampleCode)
]]            

I’m asking for two things here. I’m “tabbing” the names in four spaces, and I want the superclass to come out.

Test.

3: ProjectDescription classReport  -- Actual: Classes

SampleCode
SubclassOfSample
, Expected: Classes

    SampleCode
    SubclassOfSample(SampleCode)

Right. First the tabs. I think they belong to the collection report, not the individual item.

function ClassCollection:report()
    local tab = "    "
    result = ""
    for _i,classDef in ipairs(self.classDefinitions) do
        result = result..tab..classDef:report().."\n"
    end
    return result
end

Test. Should look better but be missing the superclass.

3: ProjectDescription classReport  -- Actual: Classes

    SampleCode
    SubclassOfSample
, Expected: Classes

    SampleCode
    SubclassOfSample(SampleCode)

Yes. Now:

function ClassDefinition:report()
    local result = self.className
    if self.superclassName then
        result = result.."("..self.superclassName..")"
    end
    return result
end

3: ProjectDescription classReport  -- Actual: Classes

    SampleCode
    SubclassOfSample
, Expected: Classes

    SampleCode
    SubclassOfSample(SampleCode)

How about that! We didn’t get the superclass name out. Are we collecting it?

function PatternDictionary:classDefinition()
    return "%s*(%w+)%s*=%s*class%(%w*%)"
end

It’s there in the pattern. Do we have a test for that?

        _:test("Find a subclass", function()
            tab = " SubclassOfSample = class(Sample)"
            local pattern = pd:classDefinition()
            local start,stop,capture = tab:find(pattern)
            _:expect(capture).is("SubclassOfSample")
        end)

That test doesn’t look for the “Sample” part.

        _: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)

Test. Fails!

4: Find a subclass  -- Actual: nil, Expected: Sample

Better review that pattern more carefully.

function PatternDictionary:classDefinition()
    return "%s*(%w+)%s*=%s*class%(%w*%)"
end

Ah. No capture parens!

function PatternDictionary:classDefinition()
    return "%s*(%w+)%s*=%s*class%((%w*)%)"
end

Test. Find a subclass runs (but we have issues to deal with) but our formatting test fails:

3: ProjectDescription classReport  -- Actual: Classes

    SampleCode()
    SubclassOfSample(SampleCode)
, Expected: Classes

    SampleCode
    SubclassOfSample(SampleCode)

The layout makes me think that there may an extra return in the expected. What’s the test?

        _:test("ProjectDescription classReport", function()
            local desiredReport = [[Classes

    SampleCode
    SubclassOfSample(SampleCode)
]]
            local report = ProjectDescription("SampleStatsProject"):classReport()
            _:expect(report).is(desiredReport)
        end)
        
    end)
end

Well, as I understand ]] there shouldn’t be two returns there, but I’ll try the obvious:

3: ProjectDescription classReport  -- Actual: Classes

    SampleCode()
    SubclassOfSample(SampleCode)
, Expected: Classes

    SampleCode
    SubclassOfSample(SampleCode)

Oh. Duh. It’s the parens on SampleCode. I think we have an empty string for the superclass.

function ClassDefinition:report()
    local result = self.className
    if self.superclassName and self.superclassName ~= "" then
        result = result.."("..self.superclassName..")"
    end
    return result
end

Let’s commit: class report correctly formatted.

However, we have issues in the pattern. What if someone were to say

Foo = class ( Bar )

Those nice pretty spaces will break our pattern. Let’s beef up the test a bit.

        _: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)

This should fail, because of the spaces I added. And it does. Improve the pattern:

function PatternDictionary:classDefinition()
    return "%s*(%w+)%s*=%s*class%s*%(%s*(%w*)%s*%)"
end

Spaces all around, put ‘em on my tab.

Commit: class recognition pattern allows more whitespace.

While it’s on my mind, let’s check the method one as well.

        _:test("Find a method", function()
            local pattern = pd:methodDefinition()
            local start,stop,className,methodName, args = tab:find(pattern)
            _:expect(className,"className").is("SampleCode")
            _:expect(methodName,"methodName").is("init")
            _:expect(args,"args").is("x")
        end)

This is weak. It depends on the contents of the tab, which we can’t see in the test. Let’s do another example using a literal string.

        _:test("Find a spaced method", function()
            local tab = "function  Mumble : foo ( bar, baz )"
            local pattern = pd:methodDefinition()
            local start,stop,className,methodName, args = tab:find(pattern)
            _:expect(className,"className").is("Mumble")
            _:expect(methodName,"methodName").is("foo")
            _:expect(args,"args").is("bar, baz")
        end)

Run the tests.

9: Find a spaced method className -- Actual: nil, Expected: Mumble
9: Find a spaced method methodName -- Actual: nil, Expected: foo
9: Find a spaced method args -- Actual: nil, Expected: bar, baz

Oh for three. Pattern is:

function PatternDictionary:methodDefinition()
    return "%s*function%s*(%w+):(%w+)%((.-)%)"
end

Ah, interesting. The args pickup is scarfing everything between the parens. I guess we can manage that. Let’s try this:

function PatternDictionary:methodDefinition()
    return "%s*function%s*(%w+)%s*:%s*(%w+)%(%s*(.-)%s*%)"
end

Test. Still fails, same way. I hate patterns when they get tricky like this.

I test by removing all the spaces from my test method and then putting them in one at a time until I get the test to fail. It fails on the space ahead of the args paren.

Missed that in the pattern. Add it:

function PatternDictionary:methodDefinition()
    return "%s*function%s*(%w+)%s*:%s*(%w+)%s*%(%s*(.-)%s*%)"
end

Keep testing … Test runs now with all the spaces in. Should be good. Commit: method definition patterns accept legal whitespace.

I think that’ll do for the day. Let’s sum up.

Summary

Fast and Loose

Throughout the course of this series, I’ve been playing a little fast and loose. My pattern tests were minimal, and my patterns reflected that minimality. I may have at one point had in mind improving them, but if I did, I forgot and then re-remembered.

I wrote code that I didn’t need yet, and mostly got away with it, but if I recall correctly, yesterday or the day before I had at least one misspelled member variable that wasn’t discovered until a later test failed.

And the steps were larger than I’d really prefer. Often I’d create whole (tiny) classes driven by one simple test. I didn’t run the tests as often as I might, which meant that I was creating code that wasn’t really directly driven by the test, and often, when I did get back to the test, the failure would surprise me, because I was keeping more in my head than I could reliably manage.

No, I’m not beating myself up. I’m reporting to myself–and to you–what I did and what happened. At the time, to quote Sarek, it seemed the logical thing to do. In retrospect, I think less fast, less loose, “many more much smaller steps” might have been better.

All’s well that ends well, and I think that what we have is pretty decent.

Except:

Format testing’s a pain

The format testing needs work, because CodeaUnit’s “this big string doesn’t equal this other big string” isn’t too handy. We’ll probably want to create something better. Or maybe not. Maybe we’ll just deal with it in looking at the output. For me, there comes a time when I do that.

These things seem familiar.

There is lots of duplication. The MethodCollection and ClassCollection classes are eerily similar:

ClassCollection = class()

function ClassCollection:init(classTable)
    self.classDefinitions = classTable
end

function ClassCollection:count()
    return #self.classDefinitions
end

function ClassCollection:report()
    local tab = "    "
    result = ""
    for _i,classDef in ipairs(self.classDefinitions) do
        result = result..tab..classDef:report().."\n"
    end
    return result
end

MethodCollection = class()

function MethodCollection:init(methodTable)
    self.methodDefinitions = methodTable
end

function MethodCollection:count()
    return #self.methodDefinitions
end

function MethodCollection:methodsForClass(className)
    local result = {}
    for _i,meth in ipairs(self.methodDefinitions) do
        if not className or meth:inClass(className) then
            table.insert(result, meth)
        end
    end
    return MethodCollection(result)
end

We can probably find something to do about those similarities. There are similarities in the class and method definition classes as well, which are really not much smarter than a simple record.

ClassDefinition = class()

function ClassDefinition:init(className,superclassName)
    self.className = className
    self.superclassName = superclassName
end

function ClassDefinition:report()
    local result = self.className
    if self.superclassName and self.superclassName ~= "" then
        result = result.."("..self.superclassName..")"
    end
    return result
end

MethodDefinition = class()

function MethodDefinition:init(className, methodName, args)
    self.className = className
    self.methodName = methodName
    self.args = args
end

function MethodDefinition:inClass(className)
    return self.className == className
end

The similarities among all these are not quite at the level of exact duplication, but they seem to follow a pattern. That pattern should perhaps be brought to the fore, to become more visible.

Aren’t you reading the tabs a lot?

Yes, we are. We read every tab once per pattern we look for. That’s already twice as many reads as are needed. We should sort that out so that each tab is read at most once.

Are things in the order you want?

Almost certainly not. We’ll want to add sorting to the collections. Increasing alphabetic, certainly. We might also want some kind of tab-based listing. But this comes under the heading of new stories. Right now we don’t have the requirement in a test.

Could querying be generalized?

We don’t have very many queries yet, but if we do generate more than a couple more, it might make sense to generalize our approach to them.

Pattern decoding seems odd.

There’s no connection in the code between the pattern, with some number of captures, and the use of the return from the gmatch call. I am inclined to create some kind of Pattern class that holds a class, and gmatches against a string, perhaps even checking to be sure it has the right number of captures.

I expect that will be fun, and probably fruitful.

Bottom Line

We’re back with a structure that I like better, and with a bit more visible capability than we had yesterday or the day before. So that’s good.

See you next time!