GitHub Repo

Can we remove the duplication of the action methods from World and Room? Let’s try.

We have this code, completely identical, in World and Room:

class World {
    var actions: Actions ...

    // DSL
    fun action(verb: String, noun: String, action: Action) {
        actions.add(Phrase(verb, noun), action)
    }

    fun action(verb: String, action: Action) {
        actions.add(Phrase(verb), action)
    }

    fun action(actions: List<String>, action: Action) {
        actions.forEach { makeAction(it, action) }
    }

    fun makeAction(command:String, action: Action) {
        val words = command.lowercase().split(" ")
        when (words.size) {
            1 -> action(words[0], action)
            else -> action(words[0], words[1], action)
        }
    }

It seems clear that these methods can be implemented on the Actions class. It looks like this:

class Actions() {
    private val map: ActionMap = mutableMapOf<Phrase,Action>()
    fun act(imperative: Imperative) {
        val action: (Imperative) -> Unit = find(imperative)
        action(imperative)
    }

    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()) }
            }
        }
    }

    fun add(phrase: Phrase, action: (Imperative) -> Unit) {
        map[phrase] = action
    }
}

Let’s do this the dull way (I feel dull today) by just copying them over first.

With the methods moved over and adjusted, it seems that the ones in World and Room can call the ones in Actions.

In World, it comes down to this:

    fun action(verb: String, noun: String, action: Action) {
        actions.action(verb, noun, action)
    }

    fun action(verb: String, action: Action) {
        actions.action(verb, action)
    }

    fun action(actions: List<String>, action: Action) {
        action(actions, action)
    }

Now it seems to me that Kotlin should be able to do better than just permit me to implement all the methods, forwarding to the actions member variable, but I’m darned if I can figure out how to do it any other way.

I’ve tried creating an interface IActions, with overrides in Actions class, but haven’t been able to get it accepted.

Finally, I try moving actions from a random val in Room, to this:

interface IActions {
    fun act(imperative: Imperative)
    fun action(verb: String, noun: String, action: Action)
    fun action(verb: String, action: Action)
    fun action(commands: List<String>, action: Action)
    fun add(phrase: Phrase, action: (Imperative) -> Unit)
}

class Actions() : IActions {
	...
    override fun act(imperative: Imperative) {
        val action: (Imperative) -> Unit = find(imperative)
        action(imperative)
    }
    ... and so on ...
class Room(val roomName:R, private val actions:IActions = Actions()): IActions by actions {

That allows me to remove the various action functions from Room and the tests all run. Commit: Room has IActions by delegation.

I’d like to do the same thing with World, but there’s an issue. A few issues, in fact.

When I make the corresponding change to World, a number of compiler errors arise.

class World( val actions: IActions = makeActions()) {

The first error is that I don’t seem to be able to call makeActions from here. Maybe I could do an init?

After quite a bit of messing about, myu tests are green. Here’s how World turned out:

class World( val actions: IActions = Actions()) :IActions by actions {
    init {
        makeActions()
    }

    private fun makeActions() {
        with(actions) {
            add(Phrase("go")) { imp: Imperative -> imp.room.move(imp, imp.world) }
            add(Phrase("say", "wd40")) { imp: Imperative ->
                imp.world.say("Very slick, but there's nothing to lubricate here.")
            }
            add(Phrase("say")) { imp: Imperative ->
                imp.world.say("Nothing happens here!")
            }
            add(Phrase("take")) { imp: Imperative -> imp.room.take(imp, imp.world) }
            add(Phrase("inventory")) { imp: Imperative -> imp.world.showInventory() }
            add(Phrase("look")) { imp: Imperative -> imp.room.look() }
            add(Phrase()) { imp: Imperative -> imp.room.unknown(imp, imp.world) }
        }
    }

That made many things work. But the tests wanted to replace the actions, so they got changed:

class ImperativeTest {
	...
    private fun getActions() = makeTestActions()
    ...

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

I “just” changed the makeTestActions to clear any existing table, and add in its own test elements. That has all the tests running.

Let me commit lest I forget. World gets action functions by delegation to actions.

It appears that the main trick is that you can delegate to a member variable in the class constructor, but not to a variable defined internally. At least I couldn’t find a way to do it.

In World, I had to init the thing separately, in an init.

class World( val actions: IActions = Actions()) :IActions by actions {
    init {
        makeActions()
    }

And, of course, makeActions doesn’t replace actions, it just fills it in. Along the way somewhere I had to implement clear, for the ImperativeTests, and put it in the IActions interface blah blah.

All in all, I like this, and I like that I’ve finally managed to get an example of delegation by an interface to compile and run.

Summary

We’ve moved from a situation where we had the same handful of methods implemented in two classes, to having the real implementations once, where they always belonged, in Actions class, and the two user classes forwarding manually to them. And then, to forwarding them by delegation, completely removing the definitions from the user classes, other than the “by” keyword.

The main advantage of all this was the reduction of a handful of identical methods in two classes to a single declaration in each class. The actual operations were moved over to the Actions class, which is, frankly, where they should have been from the beginning.

A very nice change, I think.

Good stuff, and I’ve learned a new thing. A pleasant afternoon.

I think that there may be benefit to looking more deeply at the tests, but they are working fine, so if they are a bit tricky to have set up, they’re OK now and don’t need changing.