Kotlin 72 - A Wild Advantage Appears!

GitHub Repo

Wrapping collections keeps the tricky stuff inside and offers nice clean easy stuff on the outside. That’s why we do it, right there.

It is 0530. Awakened by a thunderstorm, I have an idea that I want to try, and to write about. It’ll show, I think, why wrapping collections is usually a good idea.

When you take something in the game, the current code looks like this:

    val take = { imperative: Imperative, world: World ->
        val item = contents.remove(imperative.noun)
        if ( item!=null ) {
            world.addToInventory(item)
            world.response.say("${item.name} taken.")
        } else {
            world.response.say("I see no ${imperative.noun} here!")
        }
    }

Yesterday, we created the Items collection, used for two purposes, holding things that are in a Room, and things that are in the player’s inventory. The latter is held in the World at present, because our Player object is still quite rudimentary.

This morning, amid the sounds of rain and thunder, I had a (partial) insight. Refining it here on the fly, I’d say that there is an “invariant” that we’d like to enforce in the game: every object is either in a room, or in the inventory, never neither, never both.

(My more vague initial thought was something like “if Items had a moveTo method that succeeds or fails, take and drop would be simplified”.)

Now that I’m nominally awake, I want to try this idea to see what happens.

First, let’s do a little test on our Items object, to drive out the method.

    @Test
    fun `moving items`() {
        val axe = Item("axe")
        val inventory = Items()
        val roomContents = Items()
        roomContents.add(axe)
        assertThat(inventory.contains("axe")).isEqualTo(false)
        var done:Boolean = roomContents.moveItemTo("axe", inventory)
        assertThat(inventory.contains("axe")).isEqualTo(true)
        assertThat(done).isEqualTo(true)
        done = roomContents.moveItemTo("axe", inventory)
        assertThat(done).isEqualTo(false)
    }

Inventory originally does not have the axe, we move it from room to inventory, then inventory does contain axe, we try to move it again, and our move fails, because it isn’t in the room any more.

This won’t compile, because there is no moveItemTo. Our mission is to create it. IDEA will help.

The first implementation of moveItemTo that I find acceptable is this one:

    fun moveItemTo(name: String, target: Items): Boolean {
        val maybeItem: Item? = remove(name)
        return when (maybeItem) {
            null -> false
            else -> {
                target.add(maybeItem)
                return true
            }
        }
    }

If we don’t have the item. we return false. If we have it, we move it and return true. Simples!

The test is green. Now, remember this?

    val take = { imperative: Imperative, world: World ->
        val item = contents.remove(imperative.noun)
        if ( item!=null ) {
            world.addToInventory(item)
            world.response.say("${item.name} taken.")
        } else {
            world.response.say("I see no ${imperative.noun} here!")
        }
    }

It becomes this:

    val take = { imperative: Imperative, world: World ->
        if ( contents.moveItemTo(imperative.noun, world.inventory) ) {
            world.response.say("${imperative.noun} taken.")
        } else {
            world.response.say("I see no ${imperative.noun} here!")
        }
    }

Test. Green. Commit: Items.moveItemTo simplifies “take”.

We do not presently have a “drop” command. It’s easy to see that when we do, it will look just like this, only from inventory to contents. It’ll use the same code in Items.

What’s the Point?

The point is, it was previously possible, indeed easy, to remove something from contents and forget to put it into inventory, or to remove something from inventory and forget to put it in a room somewhere. Now, you can move things, ensuring that both the remove and add are done appropriately.

We should actually look now at the remove method:

    fun remove(key:String): Item? {
        return map.remove(key)
    }

There is just one access to this method, up in move. We can make it private, and we should. What about some of these other methods? Here’s Items:

class Items(private val map: ItemMap = mutableMapOf<String, Item>()) {
    fun size() = map.keys.size

    fun add(item: Item) {
        map[item.name] = item
    }

    fun asCarried(): String {
        return map.values.joinToString(
            prefix = "You have ",
            transform = { it.shortDesc },
            separator = ", ",
            postfix = ".\n"
        )
    }

    fun asFound(): String {
        return map.values.joinToString(separator = "") { it.asFound() }
    }

    fun contains(item: String): Boolean {
        return map.containsKey(item)
    }

    fun getItem(name: String): Item? {
        return map[name]
    }

    fun moveItemTo(name: String, target: Items): Boolean {
        val maybeItem: Item? = remove(name)
        return when (maybeItem) {
            null -> false
            else -> {
                target.add(maybeItem)
                return true
            }
        }
    }

    private fun remove(key:String): Item? {
        return map.remove(key)
    }
}

There’s one use of getItem, in a test. Here it is:

    @Test
    fun `initial item`() {
        var myRoom = Room("y")
        world {
            myRoom = room("x") {
                item("axe") {
                    desc("an axe", "an ornate axe belonging to the dwarf Bridget Ingridsdotter")
                }
            }
        }
        val item: Item = myRoom.contents.getItem("axe")!!
        assertThat(item.shortDesc).isEqualTo("an axe")
        assertThat(item.longDesc).contains("Bridget Ingridsdotter")
    }

We can get rid of the need for getItem by saving the item when we create it:

    @Test
    fun `initial item`() {
        var myItem = Item("nothing")
        world {
            room("x") {
                myItem = item("axe") {
                    desc("an axe", "an ornate axe belonging to the dwarf Bridget Ingridsdotter")
                }
            }
        }
        assertThat(myItem.shortDesc).isEqualTo("an axe")
        assertThat(myItem.longDesc).contains("Bridget Ingridsdotter")
    }

Now there are no calls to getItem and we can remove it. Green. Commit: Remove use of getItem and the method.

Hammering the Idea Home

Because we have wrapped our maps in an object of our own, Items, we have gained the following things:

There is no way to lose an object, removing it from somewhere and forgetting to put it somewhere else.
It’s easy to change the internal structure of Items without changing calling code;
We’ve reduced the complexity of calling code;
It’s much easier to write correct code, much harder to write incorrect code;
We’ve reduced the total lines of code;
We get easy access to new ideas like “move item to”.

All this goodness came from wrapping one simple map in an even simpler collection of our own.

Bottom Line: Wrapping collections keeps the tricky stuff inside and offers nice clean easy stuff on the outside. That’s why we do it, right there.