GitHub Repo

Back to the Phrase-matching find. I’ll grow it incrementally this time … and it works!

Here’s find:

    private fun find(imperative: Imperative): Action {
        return map.getOrElse(Phrase(imperative.verb)) { map.getValue(Phrase())}
    }

Here’s the one in the test, that I’m trying to use here:

    private fun find(p:Phrase, m: Map<Phrase, String>): String {
        return m.getOrElse(p) {
            m.getOrElse(p.asVerb()) {
                m.getOrElse(p.asNoun()) {
                    m.getOrElse(p.asEmpty()) {"nothing"}
                }
            }
        }
    }

I want to start with a Phrase and get the alternatives from it. Extract the function:

    private fun find(imperative: Imperative): Action {
        val p = Phrase(imperative.verb)
        return map.getOrElse(p) { map.getValue(Phrase())}
    }

That’s option-command-V, by the way. It’s time I learned some of the handy keystrokes, so I’ll start looking in the menu but then using the keys. ANd mentioning them so as to drill them into my head.

We want to start with the full phrase, but we also don’t want to break any tests. So let me do this:

    private fun find(imperative: Imperative): Action {
        val p = Phrase(imperative.verb, imperative.noun)
        return map.getOrElse(p.asVerb()) { map.getValue(Phrase())}
    }

This should run green. Yes. Commit: refactoring Actions.find. Now let me put in one level of the other find:


    private fun find(imperative: Imperative): Action {
        val p = Phrase(imperative.verb, imperative.noun)
        return map.getOrElse(p) { map.getOrElse(p.asVerb()) {map.getValue(Phrase())}}
    }

That isn’t red-lined by IDEA, so it is likely OK. Let me test and if it’s OK I’ll reformat it for us. Green. Can IDEA reformat? Not to my liking. option-shift-L though. Here’s my way:

    private fun find(imperative: Imperative): Action {
        val p = Phrase(imperative.verb, imperative.noun)
        return map.getOrElse(p) {
            map.getOrElse(p.asVerb()) {
                map.getValue(Phrase())
            }
        }
    }

Green. Commit: save point. Add the remaining clause:

    private fun find(imperative: Imperative): Action {
        val p = Phrase(imperative.verb, imperative.noun)
        return map.getOrElse(p) {
            map.getOrElse(p.asVerb()) {
                map.getOrElse(p.asNoun()) {
                    map.getValue(Phrase()) }
            }
        }
    }

We seem to be green. Commit: save point.

Now. That was all refactoring. It has changed the design but not the function of the find operation. As such, it didn’t need new tests, and I didn’t need the distraction of one that might not be running. However, we really should test the other cases. I think we’ll want to put that into ImperativeTest.

It already has a table just for testing, which derailed me a while back, since I had forgotten that it was there, with a different format. Here it is now:

private val TestActionTable = mutableMapOf(
    Phrase("go") to { imp: Imperative -> imp.testingSay("went ${imp.noun}")},
    Phrase("say") to { imp: Imperative -> imp.testingSay("said ${imp.noun}")},
    Phrase("inventory") to { imp: Imperative -> imp.testingSay("You got nothing")},
    Phrase() to { imp: Imperative -> imp.testingSay("I can't ${imp.verb} a ${imp.noun}") }
)//.withDefault { { imp: Imperative -> imp.testingSay("I can't ${imp.verb} a ${imp.noun}") }}

We should remove the default comments. We’re well past the need to put them back, I think.

Let’s write a test against that table, checking the matches for both, very only, noun only, and default. There’s a test in there that works. I’ll build on that structure. Since the current table doesn’t have take or cows, we should be able to use all forms with impunity.

    @Test
    fun `new phrase finding handles all cases`() {
        val imperatives  = getFactory()
        var imp: Imperative
        imp = imperatives.fromTwoWords("take", "cows")
        assertThat(imp.act(testLex()))
            .isEqualTo("no cows for you")
        imp = imperatives.fromTwoWords("hassle","cows")
        assertThat(imp.act(testLex()))
            .isEqualTo("please do not bug the cows")
        imp = imperatives.fromTwoWords("pet","cows")
        assertThat(imp.act(testLex()))
            .isEqualTo("what is it with you and cows?")
        imp = imperatives.fromTwoWords("hassle","bats")
        assertThat(imp.act(testLex()))
            .isEqualTo("please do not bug the bats")
        imp = Imperative("forge", "sword", world, room)
        assertThat(imp.act(testLex())).isEqualTo("I can't forge a sword")
    }

This should get close. I am irritated by the shift in the middle of my objects between Imperative to Phrase and back. We need to think about that once we’re satisfied with the Phrase implementation.

Test fails, of course, but I’ve not yet added my new elements to the table.

private val TestActionTable = mutableMapOf(
    Phrase("take", "cows") to {imp: Imperative 
        -> imp.testingSay("no cows for you")},
    Phrase("hassle") to { imp: Imperative 
        -> imp.testingSay("please do not bug the ${imp.noun}")},
    Phrase(noun="cows") to {imp:Imperative 
        -> imp.testingSay("what is it with you and cows?")},
    Phrase("go") to { imp: Imperative 
        -> imp.testingSay("went ${imp.noun}")},
    Phrase("say") to { imp: Imperative 
        -> imp.testingSay("said ${imp.noun}")},
    Phrase("inventory") to { imp: Imperative -> imp.testingSay("You got nothing")},
    Phrase() to { imp: Imperative 
        -> imp.testingSay("I can't ${imp.verb} a ${imp.noun}") }
)

Test passes. Commit: New Phrase-match find works in Actions. Time to reflect and then, er, look forward. Whatever the opposite of reflect is.

Reflection

The feature went in perfectly smoothly. Realizing that I had different defaults in the test table and the the world’s table was the key to that. It would be better if I realized more things, but I am only as smart as I am, no smarter, and often, sadly, less smart. Such is the fate of humanity.

Let’s think about this match thing. It’s really a bit odd, I think.

    private fun find(imperative: Imperative): Action {
        val p = Phrase(imperative.verb, imperative.noun)
        return map.getOrElse(p) {
            map.getOrElse(p.asVerb()) {
                map.getOrElse(p.asNoun()) {
                    map.getValue(Phrase()) }
            }
        }
    }

We must always have both a verb and a noun in the input Imperative. Those are required fields, so we can be sure they’re there, but if we are to fiddle with that object, we’ll want to be careful. I guess if we did go in with a partial one, the phrases would still just drop through until they found a match. It’s a bit weird, though, isn’t it?

More troubling is the requirement that you must have a Phrase with no verb or noun in your map so that the final call to getValue will work. I think that’s risky and that we should be specifying the default more explicitly rather than with an unchecked constraint on the map.

I do think this scheme will give us the ability to do some nice things in the game, but I freely grant that the additional power is speculative: we have no immediate in-game need for it. I plan to fix that soon.

The fact that the Actions are in the Lexicon is going to be a bit odd, in that the plan going forward is to use the actions in the room directly, which will mean that it makes more sense for the world actions to be in the world, not in the world’s lexicon. We’ll burn that bridge when we come to it.

Feelings
You may be getting the impression, with the words like “odd” and “weird” above, that I have some discomfort with the current design. That’s true, and I try to be sensitive to things like that. A general feeling of “that seems odd” is a good early warning that something is going awry in the design. the sooner we notice those things, the sooner we can fix them, and that means our design can stay closer to whatever ideals we may have.

For now, for today, I think we’re good. This morning’s apparent debacle was really learning by doing. That’s why I didn’t feel badly about it, and why I was willing to publish what happened. (Although, to be fair, I have to screw up pretty badly before I refuse to publish.)

Let’s call it an afternoon. See you next time!