Kotlin 93

GitHub Repo

It’s 0420 and I have an idea. I figure never pass up an idea, so here I am at the keyboard.

I was thinking at 0345, after the cat sat on me, about the idea of creating all the Rooms at the beginning of the game, by iterating the R enum. And then … if I’m not mistaken, an enum in Kotlin isn’t just a magical name for a number: it’s an instance of the enum class. In the case in hand, an instance of class R. And they can have constructors and method functions. So, we can give each R.WhatNot its own room member:

NOTA BENE

This first attempt fails because the compiler and I disagreed over something that I didn’t figure out in time to avoid a rollback. The second try goes better anyway. For best results, skip to Starting Again, below.

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    First, Second, Palace, Treasure, Clearing, Y2
    ;

    val room = Room(valueOf(name))
}

And, Voila! we have a room for every member of our enum, and when we have an instance of the enum in hand, we can get the room via it.room.

How can we smoothly get this to be useful? We currently have the Rooms object, a map from R to Room, which holds all the Room instances in the game. The live instance of Rooms is a member variable in World:

class World {
    var actions: Actions = makeActions()
    val lexicon = makeLexicon()
    val flags = GameStatusMap()
    val inventory: Items = Items()
    val name = "world"
    private val rooms = Rooms()
    val roomCount get() = rooms.size
    val roomReferences: Set<R> get() = rooms.roomReferences
    var response: GameResponse = GameResponse()

Let’s look at some key references to rooms:

    fun hasRoomNamed(name: R): Boolean = rooms.containsKey(name)

    fun roomNamedOrDefault(name: R, default: Room) :Room 
    	= rooms.getOrDefault(name, default)

    fun unsafeRoomNamed(name: R): Room = rooms.unsafeRoomNamed(name)

It seems to me that we can probably eliminate the has one, because given an R, we have the Room. The roomNamedOrDefault can also get the Room from the R instance. Same with unsafeRoomNamed.

There are just these other references:

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = Room(name)
        rooms.add(room)
        room.details()
        return room
    }

This is the room function in the DSL. It has the R, so it has the room, and need not create it or look it up or save it. It just starts adding details. That should be fine.

There are these:

    val roomCount get() = rooms.size
    val roomReferences: Set<R> get() = rooms.roomReferences

The roomReferences is there to check whether all the rooms are defined. We may still need a similar check, to see whether rooms have shown up in the DSL script. The code referencing the references (sorry) is no longer useful and is already marked as inapplicable.

I comment all that out: it’s in a test and I want to leave it as a reminder that we probably do want an integrity check on Rooms and references. I’d put it in Jira but I’m not a sociopath.

I delete the player.roomReferences and world.roomReferences definitions.

The roomCount is used in a couple of tests:

    @Test
    fun worldWithRoom() {
        val world = world {
            room(R.First) {}
        }
        assertThat(world.roomCount).isEqualTo(1)
        assertThat(world.hasRoomNamed(R.First)).isEqualTo(true)
    }

    @Test
    fun roomsWithGo() {
        val world = world {
            room(R.Woods) {
                go(D.South,R.Clearing)
            }
            room(R.Clearing) {
                go(D.North, R.Woods)
            }
        }
        assertThat(world.roomCount).isEqualTo(2)
        assert(world.hasRoomNamed(R.Clearing))
        val player = Player(world, R.Clearing)
        player.command("go n")
        assertThat(world.response.nextRoomName).isEqualTo(R.Woods)
    }

I’m sure these tests are going to go wonky during this change, but we know we don’t need the bulk of either of these tests. I’ll trim them down. If all else fails here, we’ll roll back, but this is going to work.

I just remove the first of those. Remind me to talk about tests as history, a theory that I once held.

Now roomCount isn’t accessed. Remove the line. What about hasRoomNamed? Who’s doing that? No one. It was just used in those couple of tests. Remove it.

Now roomNamedOrDefault. Two references:

    fun command(cmd: Command, currentRoom: Room): GameResponse {
        response = GameResponse(currentRoom.roomName)
        currentRoom.command(cmd, this)
        response.nextRoom = roomNamedOrDefault(response.nextRoomName, currentRoom)
        return response
    }

The one above will just turn into a direct reference because the roomName is already an R instance. For now, let’s change it:

    fun command(cmd: Command, currentRoom: Room): GameResponse {
        response = GameResponse(currentRoom.roomName)
        currentRoom.command(cmd, this)
        response.nextRoom = response.nextRoomName.room
        return response
    }

And the other reference is a test:

    @Test
    fun roomDescriptions() {
        val long = "You're somewhere with a long descriptions"
        val world = world {
            room(R.First) {
                desc("You're somewhere.", long)
            }
        }
        val room = world.roomNamedOrDefault(R.First, Room(R.Second))
        assertThat(room.longDesc).isEqualTo(long)
    }

    @Test
    fun roomDescriptions() {
        val long = "You're somewhere with a long descriptions"
        val world = world {
            room(R.First) {
                desc("You're somewhere.", long)
            }
        }
        val room = R.First.room
        assertThat(room.longDesc).isEqualTo(long)
    }

I’m just curious whether the tests would run, but let’s keep chasing getting rid of the rooms member. We’re pretty close to done, I think.

Here’s the DSL room method:

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = Room(name)
        rooms.add(room)
        room.details()
        return room
    }

We want just to use the Room that’s in the R …

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = name.room
        room.details()
        return room
    }

The rooms member in world is referenced by roomNameOrDefault, now unused. Remove that method. And by this:

    fun unsafeRoomNamed(name: R): Room = rooms.unsafeRoomNamed(name)

Whoever’s doing that needn’t. Find usages. There are 8. Here’s a live one, in Player:

class Player(private val world: World, startingName: R) {
    var currentRoom = world.unsafeRoomNamed(startingName)

We just replace that, of course with startingName.room.

I suddenly become lazy and am tempted to re-implement the method to work. But no, there are but six to change now.

Now there are no references to that method and therefore no references to the rooms member in World. Delete it. I can’t think of anything else to do. Let’s run the tests.

Out of 41 tests, 31 fail. I’m hopeful that there’s a central issue here. Hm the test says “could not initialize”. Let’s see whazzup.

The message I’m getting is that R is not an enum class. R is just this:

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    First, Second, Palace, Treasure, Clearing, Y2
    ;

    val room = Room(valueOf(name))
}

Starting Again

I’m getting no flags on any of the files, as far as I can see. Nothing for it but to roll back. Tempting to stash this, but no, I’ve learned a bunch and can do better next time around.

OK, back to green.

Ha! Tried just the definition above, no other changes, and I get the error. My hypothesis is that I can’t go around calling valueOf here in the constructor. We can and should set the name directly:

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    First, Second, Palace, Treasure, Clearing, Y2
    ;

    val room = Room(this)
}

That at least runs all the tests green. Now let’s see if I can devise a smoother way of making all this happen.

In the room DSL command:

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = name.room
        rooms.add(room)
        room.details()
        return room
    }

Now we should be creating all the rooms right in the enum and adding them into the rooms collection. I think everything should just work.

Not quite. It looks like room contents isn’t picking up what’s put in:

    @Test
    fun `can take inventory`() {
        val world = world {
            room(R.First) {
                desc("storage room", "large storage room")
                item("broom") {
                    desc("a broom")
                }
            }
        }
        val player = Player(world, R.First)
        var resultString = player.command("take broom")
        assertThat(resultString).isEqualTo("broom taken.\nlarge storage room\n")
        resultString = player.command("inventory")
        assertThat(resultString).isEqualTo("You have a broom.\n\nstorage room\n")
    }

Oh dear. Once we build all these rooms in the enum, they are there forever. The error is:

Expecting:
 <"broom taken.
large storage room
You find an axe.
You find some water.
">
to be equal to:
 <"broom taken.
large storage room
">
but was not.

We have an axe and water … because some other test has put them into this room.

The DSL room command needs to clear the Room before carrying on. I implement room.clear(), and Items.clear(), using map.clear() and the tests are green.

I’m not entirely comfortable. I’d prefer that the tests and the real game get their own instances of Room in R. I do like, however, the notion that the R element holds the room, avoiding all the lookup tables.

Maybe we can proceed another way, creating the Room instances in a batch. A day or so ago, the plan was to init the Rooms table all at one go. I like making the enum hold the rooms, because it guarantees that any room name has a valid room attached.

We can defer that issue. The clear is working. (Hmm … what if we were to tell the enum to clear rather than the room?)

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = name.room
        name.clear()
        rooms.add(room)
        room.details()
        return room
    }

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    First, Second, Palace, Treasure, Clearing, Y2
    ;

    fun clear() {
        room.clear()
    }

    val room = Room(this)
}

Yes. Now, if we want to, we can make R.clear create a whole new room instance. Now let’s see about not adding the already existing Room instances to the rooms member. We’ve been down this path before but let’s be a bit more clever this time.

class World ...
    fun hasRoomNamed(name: R): Boolean = rooms.containsKey(name)

That could return true, or people could just not ask. Both references are tests. Remove one test entirely, fix the other not to ask: we know there is a room now.

We are green. Shall we assume this is going to work and commit? Yes. It does work.

Next reference to rooms:

    fun roomNamedOrDefault(name: R, default: Room) :Room = rooms.getOrDefault(name, default)

Re-implement, or replace? Replace:

    fun command(cmd: Command, currentRoom: Room): GameResponse {
        response = GameResponse(currentRoom.roomName)
        currentRoom.command(cmd, this)
        response.nextRoom = response.nextRoomName.room
        return response
    }

    @Test
    fun roomDescriptions() {
        val long = "You're somewhere with a long descriptions"
        val world = world {
            room(R.First) {
                desc("You're somewhere.", long)
            }
        }
        val room = R.First.room
        assertThat(room.longDesc).isEqualTo(long)
    }

Test. Green. Commit: remove roomNameOrDefault, replace with R.room.

Another rooms reference:

    fun unsafeRoomNamed(name: R): Room = rooms.unsafeRoomNamed(name)

Fix those all to to name.room. Tests are green. Commit: remove unsafeRoomNamed.

Now there are only two references to rooms in World, the add in the room DSL function, and roomReferences. Let’s remove that. Again I comment out the test that refers to references.

The only remaining reference to rooms is here:

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = name.room
        name.clear()
        rooms.add(room)
        room.details()
        return room
    }

Remove it. Remove rooms. Test. Green. Commit: Rooms are now auto-defined in R enum.

Now the Rooms class seems redundant. The only reference is in its tests. Remove those. Remove Rooms. Test. Green. Commit: Remove Rooms class and its tests.

This is a great place to stop. I can go back to bed for a while. My 4 AM energy is wearing off.

It seems clear to me that if I had worked out the problem in my initial implementation of putting the Room into the enum, we’d have been green an hour ago. But this path was much cleaner than the first path, so the rollback at least had the advantage of my being confident, and having the ability to keep the tests running all the time.

So that’s nice.

Before I forget, we need to improve Room.clear() to clear all room’s member variables, or, what would be better, to create a new room at the time we send clear() to the enum instance for the room.

Later. For now, I’m tired, and I’m going back to bed. We’ll talk later, OK?

0910

After a short but refreshing nap, I’m back. I think we’ll modify the R.clear() to just replace the whole room object. That makes the R enums mutable, but it’s a controlled kind of mutation. I think that at some future time we’ll probably put a method on the class R itself to clear them all at once, but for now, we’re clearing as needed.

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    First, Second, Palace, Treasure, Clearing, Y2
    ;
    var room = Room(this)

    fun clear() {
        room = Room(this)
    }
}

Test. Curiously, replacing the room that way doesn’t work. A raft of tests fail. I need to know why.

Here’s the simplest one I can find:

    @Test
    fun roomDescriptions() {
        val long = "You're somewhere with a long descriptions"
        world {
            room(R.First) {
                desc("You're somewhere.", long)
            }
        }
        val room = R.First.room
        assertThat(room.longDesc).isEqualTo(long)
    }

Oh. I know. It’s this:

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = name.room
        name.clear()
        room.details()
        return room
    }

We’d do well to use the new room, not the old one.

    fun room(name: R, details: Room.()->Unit) : Room {
        val room = name.freshRoom()
        room.details()
        return room
    }

    fun freshRoom(): Room {
        room = Room(this)
        return room
    }

Green. Note that I renamed the clear to freshRoom. It made more sense to me to have the function return the room. Now I think no one calls Room.clear(). Remove it. Slightly simpler.

I think we should make the room member in R be read-only from the outside. With a little help from IDEA:

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    First, Second, Palace, Treasure, Clearing, Y2
    ;

    var room = Room(this)
        private set

    fun freshRoom(): Room {
        room = Room(this)
        return room
    }
}

What a clever compiler! Test. Commit: R.freshRoom() ensures a fresh room.

Let’s reflect.

Reflection

Building the Room directly into the R enum took two passes, because I did something that made Kotlin explode, and didn’t see at first what it was. I’ll see about filing a bug report on it.

The rollback wasn’t necessary, but the second time through I went much more smoothly, and never had the tests broken for more than a moment. In fact, it would have been possible to recast the methods like hasRoomNamed and unsafeRoomNamed to work on the new structure, and I could have left all the referencing code in place, to be replaced over a longer period of time. If this were a library, we could have deprecated those functions and replaced them N versions from now.

We’ve removed half a dozen methods at least, plus one whole class and its tests, the Rooms class. We have the equivalent capability in less code, and it’s faster too, not that that matters.

I’d prefer the R things to be immutable, but if I’m going to test against them, and I am, I don’t see how to manage that. Even so, the fact that there is guaranteed to be a Room for every R is a Good Thing.

One slightly irritating thing is that there are some elements of the R enum that are there only for the tests. They include First, Second, Palace and Treasure. There may be some way to make a different R available to the tests than to the game, but given that the tests can see the whole package, I don’t see how to do it. Maybe we’ll talk about it Tuesday, or maybe a reader will suggest something. For now, I’m going to rename all those Rs to make it clear that they are for testing.

I wish “test” started with Z. Hm, can they start with underbar? Well, I get a weak warning telling me that it should start with an upper-case letter. OK then … Z_First? Now the warning is:

Enum entry name 'Z_First' doesn't match regex '[A-Z]([A-Za-z\d]*|[A-Z_\d]*)'

I guess the rule is if you want an underbar you have to have all caps in the enum name. Swell.

With the name ZtestFirst, it warns me that there is a typo in the name. It would accept ZTestFirst, I bet. Yes it will. Sheesh. We compromise on this:

enum class R {
    Spring, Wellhouse, Woods, WoodsNearCave, CaveEntrance,
    Clearing, Y2,
    ZTestFirst, ZTestSecond, ZTestPalace, ZTestTreasure,
    ;

That’ll keep them out of the way except when I create a room named Zoo. Test. Commit: Rename test R enum members to ZZTestEtcEtc.

Late Update

Having figured out the regex, I rename the test names to things like Z_FIRST, all upper case. I leave the others CamelCase.

Summary

I think this is better. The use of the R enum as a container for the Room is a bit odd, but my last use of enum was the kind that just represents an integer. These are more capable than that, and aside from the mutability, I think this is a righteous usage.

I’ll wrap this and go look like someone who needs Sunday breakfast. The cat sits on the kitchen table when she’s hungry. Maybe I’ll try that. Oh, and I promised this:

Tests as History

I used to think that the microtests, left in chronological order, could tell the story of how an object came into being and how it evolved. And, in fact, I’ve seen them used that way. But we’re well past that situation here, because we’ve just made yet another big change to the system’s design, moving the Room inside the R enum, and of course the R enum itself changed us away from naming so many things with plain strings.

So I’m letting go of that idea, to the extent that I was still holding on to it. I’ll feel more free to remove and recast tests with that idea off the table.

All in all, a noticeable improvement! See you next time!