How about a bit of code improvement? That might be fun.

I’m thinking that the world’s collection of rooms should be a map from name to room; that the Game should know the current room, not just the room name, and that the world should keep track of names referred to in go commands, so that we can make sure the world is well-formed. I suspect there will be other similar checks to be made as we go along.

Oh, and I’m not sure what we should do if there is an attempt to define a room with the same name as an existing one. I don’t want to be throwing exceptions or anything fatal. Perhaps ignore the new one, or return the existing one? I wonder if we could allow that so that after you’ve defined a bunch of things on room X, you can say room(X) again and define more stuff on it. That could certainly work.

I’ll start with the rooms being a map.

Rooms as Map

We have this:

class World {
    val name = "world"
    val rooms = mutableListOf<Room>()

    fun room(name: String, init: Room.()->Unit) : Room {
        val room = Room(name)
        rooms += room
        room.init()
        return room
    }
}

And we want a mutable map. And we have some tests that probably depedend too much on how it works, like this one:

    @Test
    internal fun roomsWithGo() {
        val world = world {
            room("woods") {
                go("s","clearing")
            }
            room("clearing") {
                go("n", "woods")
            }
        }
        assertThat(world.rooms.size).isEqualTo(2)
        assertThat(world.rooms[1].name).isEqualTo("clearing")
        val r1Moves = world.rooms[1].moves[0]
        val expected = Pair("n","woods")
        assertThat(r1Moves).isEqualTo(expected)
    }

I think we should be asking questions of the world, rather than rippig its guts out. Let’s TDD in some new properties:

        assertThat(world.roomCount).isEqualTo(2)

And implement that …

    val roomCount = rooms.size

I think that ought to work. Test. No. Try this:

    val roomCount get() = rooms.size

Yes, that works. Use it in the other test. Still green. Commit: implement roomCount dynamic property on World.

Now what about this test:

    @Test
    fun worldWithRoom() {
        val world = world {
            room("living room") {}
        }
        assertThat(world.roomCount).isEqualTo(1)
        assertThat(world.rooms[0].name).isEqualTo("living room")
    }

We’d like to ask the world whether it has a room named “living room”, and perhaps to return it. I think I want the predicate as well as the getter.

        assertThat(world.hasRoomNamed("living room")).isEqualTo(true)

And perhaps …

    fun hasRoomNamed(name: String): Boolean {
        return rooms.any { it.name == name}
    }

Test, with hope if not confidence … Green! Woot! Replace the other calls:

        assert(world.hasRoomNamed("clearing"))

I used the simple assert here rather than isEqualTo(true). Green.

Now we have this bit:

        val r1Moves = world.rooms[1].moves[0]
        val expected = Pair("n","woods")
        assertThat(r1Moves).isEqualTo(expected)

I’m really just trying to determine whether the moves in the clearing include moving north to the woods. I do think that the world should be able to answer that, and that it should defer the question to the room. Which means we need a room getter, among other things.

Let’s recast the test:

        assertThat(world.roomCount).isEqualTo(2)
        assert(world.hasRoomNamed("clearing"))
        val clearing = world.roomNamed("clearing")
        val newLocName = clearing.move("n")
        assertThat(newLocName).isEqualTo("woods")

I was thinking to return the room, but the room doesn’t know other rooms, just their names, so move will have to return a name.

So we implement, in World:

    fun roomNamed(name:String) :Room {
        return rooms.first {it.name == name}
    }

And in Room:

    fun move(direction: String) :String {
        return moves.first { it.first == direction}.second
    }

Now these will do bad things if we ever ask for things that aren’t there, so we’ll add that to our list of things to work on.

Now it seems to me that I would like to make rooms private in World, but right now we can’t. We can commit what we have, however. Commit: tests no longer rip guts out of world object.

Improving Game

Game assumes that the starting room is room 0, and fact is, we want the world’s rooms to be more private than that and probably the game should be given the starting room when created. Let’s change that:

    @Test
    internal fun gameCheck() {
        val world = world {
            room("woods") {
                go("s","clearing")
            }
            room("clearing") {
                go("n", "woods")
            }
        }
        val game = Game(world, "woods")
        assertThat(game.currentRoomName).isEqualTo("woods")
        game.command("s")
        assertThat(game.currentRoomName).isEqualTo("clearing")
        game.command("n")
        assertThat(game.currentRoomName).isEqualTo("woods")
    }

We need to change the constructor:

class Game(val world: World, val startingName: String) {
    var currentRoomName = startingName
    ...

Tests should still run … and they do.

I think we should be able to change the underlying form of rooms now. Can we declare them private? Not quite, Game has this:

    fun currentRoom(): Room {
        return world.rooms.first{it.name == currentRoomName}
    }

But we have roomNamed now, so:

    fun currentRoom(): Room {
        return world.roomNamed(currentRoomName)
    }

And rooms in World can be private. Now let’s change it to a map.

    private val rooms = mutableMapOf<String,Room>()

And a few other changes …

    fun hasRoomNamed(name: String): Boolean {
        return rooms.containsKey(name)
    }

    fun roomNamed(name:String) :Room {
        return rooms[name]!!
    }

The fetch there is a “non-null assertion” that will throw if we don’t find name. I don’t like that but first it has to work. Kotlin tells me I have to do this:

    fun room(name: String, init: Room.()->Unit) : Room {
        val room = Room(name)
        rooms[name] = room
        room.init()
        return room
    }

Used to have rooms+=room there. Tests are green. Commit: world’s rooms are now a mutable map not list.

Idea warns me:

Warning:(3, 30) Constructor parameter is never used as a property

True enough. We can remove teh val from the second parm in Game:

class Game(val world: World, startingName: String) {
    var currentRoomName = startingName

Commit again: game second parm no longer val.

There’s more to do, but it’s about 1600, and I want to rest my aging eyes for the zoom meeting tonight. Let’s sum up.

Summary

Welp. We converted world.rooms to a map, and did so rather smoothly for a Kotlin beginner. It’s not entirely bulletproof yet: missing room names could cause exceptions and we’d like to code to be as exception free as possible. But all the tests are running, no one knows the format of the rooms table but World, not even the tests, which is a very good thing.

I want to learn a bit more about good practices with maps and nulls and such. Maybe that will come up tonight if we get a chance to look at my code. I think a lot of us have code to show, so we’ll see who gets more time in the barrel.

Either way, the code is a bit better, by my standards. Still room for improvement, of course.

Still, a good afternoon, and after a good morning, that’s downright super.

See you next time!