GitHub Repo

It’s 0500, way too early to be awake. I am awake. To keep the demons away, I think I’ll see about narrowing the Flyer creation interface. As one does.

The Flyer creation is more complicated than it should be. More to the point, more complicated than I want it to be. I have a tentative plan, and a desire. We’ll consider the Interface IFlyer, the Flyer constructor and companion methods, the plan, and the desire, in that order.

Interface

The flyers, both Flyer and the special ones, have this interface:

interface IFlyer {
    val killRadius: Double
        get() = -Double.MAX_VALUE
    val position: Point
        get() = Point(-666.0, -666.0)
    val mutuallyInvulnerable: Boolean
        get() = true
    val score: Int
        get() = 0
    val velocity
        get() = Velocity(0.0, 100.0)
    val elapsedTime
        get() = 0.0
    val lifetime
        get() = Double.MAX_VALUE
    fun draw(drawer: Drawer) {}
    fun interactWith(other: IFlyer): List<IFlyer>
    fun interactWithOther(other: IFlyer): List<IFlyer>
    fun move(deltaTime: Double) {}
    fun finalize(): List<IFlyer> { return emptyList() }
    fun update(deltaTime: Double): List<IFlyer>
}

The methods aren’t bad but 7 member variables seems a lot to specify in an interface. Granted, some of that is there for the tests, but whatever the reason, it’s a lot.

Creating a Flyer

Flyer is the class of things that move, asteroids, missiles, ship, and the splat, with more to come. The construction looks like this:

class Flyer(
    override var position: Point,
    override var velocity: Velocity,
    override var killRadius: Double,
    override val mutuallyInvulnerable: Boolean = false,
    val view: FlyerView = NullView(),
    val controls: Controls = Controls()
) : IFlyer {
    var heading: Double = 0.0
    var finalizer: IFinalizer = DefaultFinalizer()
    override var elapsedTime = 0.0
    override var lifetime = Double.MAX_VALUE

The individual creation methods, in a Flyer companion object, are more helpful, but each is pretty complicated:

    companion object {
        fun asteroid(pos:Point, vel: Velocity, killRad: Double = 500.0, splitCt: Int = 2): Flyer {
            return Flyer(
                position = pos,
                velocity = vel,
                killRadius = killRad,
                mutuallyInvulnerable = true,
                view = AsteroidView()
            ).also { it.finalizer = AsteroidFinalizer()}
        }

        fun ship(pos:Point, control:Controls= Controls()): Flyer {
            return Flyer(
                position = pos,
                velocity = Velocity.ZERO,
                killRadius = 150.0,
                mutuallyInvulnerable = false,
                view = ShipView(),
                controls = control,
            )
        }

        fun missile(ship: Flyer): Flyer {
            val missileKillRadius = 10.0
            val missileOwnVelocity = Velocity(SPEED_OF_LIGHT / 3.0, 0.0).rotate(ship.heading)
            val missilePos: Point = ship.position + Velocity(2*ship.killRadius + 2 * missileKillRadius, 0.0).rotate(ship.heading)
            val missileVel: Velocity = ship.velocity + missileOwnVelocity
            val flyer =  Flyer(missilePos, missileVel, missileKillRadius, false, MissileView())
            flyer.lifetime = 3.0
            flyer.finalizer = MissileFinalizer()
            return flyer
        }

        fun splat(missile: Flyer): Flyer {
            val splat = Flyer(missile.position, Velocity.ZERO, -Double.MAX_VALUE, true, SplatView())
            splat.lifetime = 2.0
            return splat
        }
    }

The Plan

My tentative plan is to move position, velocity, and heading, which is not part of the constructor, off into a new or newish object, one that stands where the Controls do at present, but that maintains and provides the Flyer position over time. We might call this object the Pilot. The basic one just keeps on going in the current direction, and will apply to almost every Flyer. A more capable one will be in the ship Flyer, wired into the controls. It is possible that the current Controls object will evolve to have the position, velocity, and so on inside it. It might even be that there will be just the one object, but I am of the opinion that having the capable one we have now, plus a trivial one, will be a better design.

A second aspect of my plan is this: although my starting purpose is to move the position and velocity and such into a new object, I am going to look at each of the required member variables and try to find ways to make them unnecessary as part of the object construction and interface, even if they are important to the objects inside. Note that most of those variables have a default value via a getter that produces a default value. That’s telling me that they’re not as important to the interface as one might think.

A third aspect — this plan is way too complicated — is that I suspect that much of the Flyer’s breadth of interface is due to the way the tests work, looking at changing properties. I propose to deal with this in at least three ways. One will be to provide Flyer extensions in the tests themselves, to get and set things. A second will be to adjust the tests in other ways, ways that are quick and easy. The third will be to change the tests to test different objects, such as the Pilot, rather than the whole object.

That’s a lot. I could list all my abilities and resources here. I plan to do what’s necessary, of course. However:

The Desire

The last few changes to Flyer have been improving it, but have taken a bit of time, typically a couple of hours, and the system has been broken for much of those hours. For what follows, I want to find “Many More Much Smaller Steps”, with each one taking just a few minutes, and each one leaving the system working perfectly.

I do not know how I’m going to do this. Let’s find out.

Down To It

We’ll scan the interface and constructor again and again, looking for little things we can quickly do. On my first scan, I notice score. Why do all the Flyers need score? The reason is a combination of Kotlin and perhaps my ignorance of all the tricks of the Kotlin trade. The ScoreKeeper needs to get the score from any Score objects that are passing by. So far, we do not check objects for type in our special objects. Here’s how ScoreKeeper interacts:

    override fun interactWith(other: IFlyer): List<IFlyer> {
        if (other.score > 0) {
            totalScore += other.score
            return listOf(other)
        }
        return emptyList()
    }

    override fun interactWithOther(other: IFlyer): List<IFlyer> {
        return this.interactWith(other)
    }

It just asks everything that goes by to provide a score. If it could be sure that the thing wasn’t a Score, it wouldn’t ask. I had hoped that I could just add in a version of interactWith that looked for Score instances, like this:

    fun interactWith(other: Score): List<IFlyer> {
        totalScore += other.score
        return listOf(other)
    }

That worked in my tests, I think because in the tests Kotlin knew that I was looking at a Score, while in the game it doesn’t check the type dynamically to select the method to be called. Bummer.

I could check the class in ScoreKeeper but that way lies perdition. Somewhere in my past I’ve learned “Never check the type of an object”. We do, however, permit multiple dispatch. But to do that, we’d just have to override a bunch of new methods like interactWithScore() all over. That way looks no good.

Since we have a default in the interface, we don’t have to worry about Score elsewhere. I’m looking at this wrong: I should really only be concerned with the members that everyone has to deal with. The rest aren’t really members so much as they are default behavior. Let’s scan again:

interface IFlyer {
    val killRadius: Double
        get() = -Double.MAX_VALUE
    val position: Point
        get() = Point(-666.0, -666.0)
    val mutuallyInvulnerable: Boolean
        get() = true
    val score: Int
        get() = 0
    val velocity
        get() = Velocity(0.0, 100.0)
    val elapsedTime
        get() = 0.0
    val lifetime
        get() = Double.MAX_VALUE
    fun draw(drawer: Drawer) {}
    fun interactWith(other: IFlyer): List<IFlyer>
    fun interactWithOther(other: IFlyer): List<IFlyer>
    fun move(deltaTime: Double) {}
    fun finalize(): List<IFlyer> { return emptyList() }
    fun update(deltaTime: Double): List<IFlyer>
}

Let’s reorder these parameters in a more sensible way. I think we can change the order of things in the Interface without bothering anyone implementing it. Maybe sorting them will give me some insight. I come up with this order and some comments to keep my thoughts more or less clear:

interface IFlyer {
    val position: Point
        // default position is off screen
        get() = Point(-666.0, -666.0) 
    val velocity
        // something magical about this number
        get() = Velocity(0.0, 100.0) 
    val killRadius: Double
        // no one can hit me
        get() = -Double.MAX_VALUE
    val mutuallyInvulnerable: Boolean
        get() = true
    
    // fake values for interactions
    val elapsedTime
        get() = 0.0
    val lifetime
        get() = Double.MAX_VALUE
    val score: Int
        get() = 0

So it would seem that our constructor needs only those top four values, even before we try to bind up position and velocity. Let’s review the constructor and companion ones again:

class Flyer(
    override var position: Point,
    override var velocity: Velocity,
    override var killRadius: Double,
    override val mutuallyInvulnerable: Boolean = false,
    val view: FlyerView = NullView(),
    val controls: Controls = Controls()
) : IFlyer {
    var heading: Double = 0.0
    var finalizer: IFinalizer = DefaultFinalizer()
    override var elapsedTime = 0.0
    override var lifetime = Double.MAX_VALUE

The whole “mutuallyInvulnerable” thing is a bit of a crock. Any object with that flag set true will not destroy any other object with that flag also set when they collide. There are two cases of interest. First and most important, asteroids do not collide with each other. Second, the special objects do not collide with each other. That’s why the flag defaults to true in the interface and false in the constructor. I’ll comment that before I forget.

It’s hard to think about this flag but I don’t see anything to improve it.

Unfortunately for our mission of improving the Flyer constructor, the finalizer should clearly be a parameter. Currently we patch it in, for example here:

        fun asteroid(pos:Point, vel: Velocity, killRad: Double = 500.0, splitCt: Int = 2): Flyer {
            return Flyer(
                position = pos,
                velocity = vel,
                killRadius = killRad,
                mutuallyInvulnerable = true,
                view = AsteroidView()
            ).also { it.finalizer = AsteroidFinalizer()}
        }

OK, I have a rule for the situation, called Complete Construction Method. That rule, which I learned long ago, says that when we construct an object, it should all be in one go, rather than our creating it and then jamming critical values into it. Obviously it’s more of a guideline than an actual rule, but it’s a good one. Move finalizer into the signature and fix the callers. (I think we’ll do the controls also, but one thing at a time.)

class Flyer(
    override var position: Point,
    override var velocity: Velocity,
    override var killRadius: Double,
    override val mutuallyInvulnerable: Boolean = false,
    val view: FlyerView = NullView(),
    val controls: Controls = Controls(),
    val finalizer: IFinalizer = DefaultFinalizer()
)

And in the companion:

        fun asteroid(pos:Point, vel: Velocity, killRad: Double = 500.0, splitCt: Int = 2): Flyer {
            return Flyer(
                position = pos,
                velocity = vel,
                killRadius = killRad,
                mutuallyInvulnerable = true,
                view = AsteroidView(),
                finalizer = AsteroidFinalizer()
            )
        }

I think I’ll expand the others to show the parameter names, it really helps.

I am now wondering why the ship does not have a special finalizer. Oh, it’s because it just goes away. It’s the ShipMonitor that brings it back. So this one is OK:

        fun ship(pos:Point, control:Controls= Controls()): Flyer {
            return Flyer(
                position = pos,
                velocity = Velocity.ZERO,
                killRadius = 150.0,
                mutuallyInvulnerable = false,
                view = ShipView(),
                controls = control,
            )
        }

And …

        fun missile(ship: Flyer): Flyer {
            val missileKillRadius = 10.0
            val missileOwnVelocity = Velocity(SPEED_OF_LIGHT / 3.0, 0.0).rotate(ship.heading)
            val missilePos: Point = ship.position + Velocity(2*ship.killRadius + 2 * missileKillRadius, 0.0).rotate(ship.heading)
            val missileVel: Velocity = ship.velocity + missileOwnVelocity
            val flyer =  Flyer(
                position = missilePos,
                velocity = missileVel,
                killRadius = missileKillRadius,
                mutuallyInvulnerable = false,
                view = MissileView(),
                finalizer = MissileFinalizer()
            )
            flyer.lifetime = 3.0
            return flyer
        }

The lifetime is an issue. Deal with it later.

I thought this would pass all the tests, but no, the asteroid splitting logic wants to provide a new finalizer:

class AsteroidFinalizer(private val splitCount:Int = 2): IFinalizer {
    private fun asSplit(asteroid: Flyer): Flyer {
        val newKr = asteroid.killRadius / 2.0
        val newVel = asteroid.velocity.rotate(Math.random() * 360.0)
        val flyer =  Flyer.asteroid(asteroid.position, newVel, newKr)
        flyer.finalizer = AsteroidFinalizer(splitCount - 1)
        return flyer

Let’s expand the parameters here. That really helps with these broad constructors. And we see that we want it to be a parameter, although maybe we should prefer the splitCount?

class AsteroidFinalizer(private val splitCount:Int = 2): IFinalizer {
    private fun asSplit(asteroid: Flyer): Flyer {
        val newKr = asteroid.killRadius / 2.0
        val newVel = asteroid.velocity.rotate(Math.random() * 360.0)
        val flyer =  Flyer.asteroid(
            pos = asteroid.position, 
            vel = newVel, 
            killRad = newKr,
            finalizer = AsteroidFinalizer(splitCount - 1)
        )
        return flyer
    }

Oh! It does deal with splitCount! So we can do this:

class AsteroidFinalizer(private val splitCount:Int = 2): IFinalizer {
    private fun asSplit(asteroid: Flyer): Flyer {
        val newKr = asteroid.killRadius / 2.0
        val newVel = asteroid.velocity.rotate(Math.random() * 360.0)
        return Flyer.asteroid(
            pos = asteroid.position,
            vel = newVel,
            killRad = newKr,
            splitCt = splitCount - 1
        )
    }

Are we green? Not quite. Grr, this is taking too long.

[should not split third time] 
expected: 1
 but was: 3

Ah. Forgot to use the split count in the asteroid builder:

        fun asteroid(pos:Point, vel: Velocity, killRad: Double = 500.0, splitCt: Int = 2): Flyer {
            return Flyer(
                position = pos,
                velocity = vel,
                killRadius = killRad,
                mutuallyInvulnerable = true,
                view = AsteroidView(),
                finalizer = AsteroidFinalizer(splitCt)
            )
        }

That should fix it. Yes. Green Commit: finalizer is now a parameter in Flyer, not an afterthought.

Now what about lifetime? Only missiles have them. Everyone else lives forever.

The rule applies. And frankly, as we make this constructor worse, we are also making it cover everything it has to, and keeping the mess in front of us. It’ll be better even now and will make room for easier changes later.

class Flyer(
    override var position: Point,
    override var velocity: Velocity,
    override var killRadius: Double,
    
    override val mutuallyInvulnerable: Boolean = false,
    override val lifetime: Double = Double.MAX_VALUE,
    val view: FlyerView = NullView(),
    val controls: Controls = Controls(),
    val finalizer: IFinalizer = DefaultFinalizer()
)

This allows and requires:

        fun missile(ship: Flyer): Flyer {
            val missileKillRadius = 10.0
            val missileOwnVelocity = Velocity(SPEED_OF_LIGHT / 3.0, 0.0).rotate(ship.heading)
            val missilePos: Point =
                ship.position + Velocity(2 * ship.killRadius + 2 * missileKillRadius, 0.0).rotate(ship.heading)
            val missileVel: Velocity = ship.velocity + missileOwnVelocity
            return Flyer(
                position = missilePos,
                velocity = missileVel,
                killRadius = missileKillRadius,
                mutuallyInvulnerable = false,
                lifetime = 3.0,
                view = MissileView(),
                finalizer = MissileFinalizer()
            )
        }

        fun splat(missile: Flyer): Flyer {
            return Flyer(
                position = missile.position,
                velocity = Velocity.ZERO,
                killRadius = -Double.MAX_VALUE,
                lifetime = 2.0,
                mutuallyInvulnerable = true, view = SplatView()
            )
        }

Test. I find one test that needs to move lifetime into the constructor. Green. Commit: lifetime added to Flyer constructor. Complete Construction Method FTW.

Commentary

This is weird. I set out to simplify the Flyer constructor. I thought that would entail removing items from it and putting them inside other classes or something. So far, I’ve added items to its constructor. However, the impact has been to simplify the companion object constructors for asteroid, missile, ship, and splat. So we have a net improvement, I think, but it is absolutely not what I expected.

The actual code told me something other than what I expected to hear. Fortunately, I listened, and both the code and I benefit.

Moving Right Along …

Now I think I see something that can be simplified:

        fun splat(missile: Flyer): Flyer {
            return Flyer(
                position = missile.position,
                velocity = Velocity.ZERO,
                killRadius = -Double.MAX_VALUE,
                lifetime = 2.0,
                mutuallyInvulnerable = true, 
                view = SplatView()
            )
        }

The default for kill radius should the same as we’re setting here. We can remove it from the call and put it into the Flyer constructor:

class Flyer(
    override var position: Point,
    override var velocity: Velocity,
    
    override var killRadius: Double = -Double.MAX_VALUE,
    override val mutuallyInvulnerable: Boolean = false,
    override val lifetime: Double = Double.MAX_VALUE,
    val view: FlyerView = NullView(),
    val controls: Controls = Controls(),
    val finalizer: IFinalizer = DefaultFinalizer()
)

I’ve put all the parameters with defaults below the blank line, which ought to make it clear that we really don’t need to supply everything. And because Kotlin will let us list provided parameters directly, I think we’re in pretty good shape. Let’s review the companion constructors again:

    companion object {
        fun asteroid(pos:Point, vel: Velocity, killRad: Double = 500.0, splitCt: Int = 2): Flyer {
            return Flyer(
                position = pos,
                velocity = vel,
                killRadius = killRad,
                mutuallyInvulnerable = true,
                view = AsteroidView(),
                finalizer = AsteroidFinalizer(splitCt)
            )
        }

        fun ship(pos:Point, control:Controls= Controls()): Flyer {
            return Flyer(
                position = pos,
                velocity = Velocity.ZERO,
                killRadius = 150.0,
                mutuallyInvulnerable = false,
                view = ShipView(),
                controls = control,
            )
        }

        fun missile(ship: Flyer): Flyer {
            val missileKillRadius = 10.0
            val missileOwnVelocity = Velocity(SPEED_OF_LIGHT / 3.0, 0.0).rotate(ship.heading)
            val missilePos: Point =
                ship.position + Velocity(2 * ship.killRadius + 2 * missileKillRadius, 0.0).rotate(ship.heading)
            val missileVel: Velocity = ship.velocity + missileOwnVelocity
            return Flyer(
                position = missilePos,
                velocity = missileVel,
                killRadius = missileKillRadius,
                mutuallyInvulnerable = false,
                lifetime = 3.0,
                view = MissileView(),
                finalizer = MissileFinalizer()
            )
        }

        fun splat(missile: Flyer): Flyer {
            return Flyer(
                position = missile.position,
                velocity = Velocity.ZERO,
                lifetime = 2.0,
                mutuallyInvulnerable = true,
                view = SplatView()
            )
        }
    }

I can remove the lines setting mutuallyInvulnerable to false, and I’m tempted to let missiles shoot each other down. Sure. Remove that one too. Here we are:

    companion object {
        fun asteroid(pos:Point, vel: Velocity, killRad: Double = 500.0, splitCount: Int = 2): Flyer {
            return Flyer(
                position = pos,
                velocity = vel,
                killRadius = killRad,
                mutuallyInvulnerable = true,
                view = AsteroidView(),
                finalizer = AsteroidFinalizer(splitCount)
            )
        }

        fun ship(pos:Point, control:Controls= Controls()): Flyer {
            return Flyer(
                position = pos,
                velocity = Velocity.ZERO,
                killRadius = 150.0,
                view = ShipView(),
                controls = control,
            )
        }

        fun missile(ship: Flyer): Flyer {
            val missileKillRadius = 10.0
            val missileOwnVelocity = Velocity(SPEED_OF_LIGHT / 3.0, 0.0).rotate(ship.heading)
            val standardOffset = Point(2 * (ship.killRadius + missileKillRadius), 0.0)
            val rotatedOffset = standardOffset.rotate(ship.heading)
            val missilePos: Point = ship.position + rotatedOffset
            val missileVel: Velocity = ship.velocity + missileOwnVelocity
            return Flyer(
                position = missilePos,
                velocity = missileVel,
                killRadius = missileKillRadius,
                lifetime = 3.0,
                view = MissileView(),
                finalizer = MissileFinalizer()
            )
        }

        fun splat(missile: Flyer): Flyer {
            return Flyer(
                position = missile.position,
                velocity = Velocity.ZERO,
                lifetime = 2.0,
                view = SplatView()
            )
        }
    }

I think this is a legitimate improvement, entirely not what I expected to have happen. Test. Green. Commit: Tidy Flyer companion object. Missiles can now destroy each other.

Now, if you’ll permit me, I plan to sum up and then return to bed for the rest of my night’s sleep.

Summary

I thought the tests were causing much of the existing complexity. That turned out not to be the case. I only had a couple of issues with tests needing to be changed. They did catch me in a mistake a time or two but they were not part of the problem. Go figure.

I did manage to make small changes and to keep the system working all the time. Some of the changes took longer than I’d have liked, but I think that may have had something to do with it being 5 AM. And I know that some of it was due to the cat bothering me. Turns out she has never been fed in her entire life and she really felt that she should have some chicken.

I set out with a plan, moving position, velocity, heading, and acceleration into a helper object, to be named Pilot or something. The overall objective was to improve construction of Flyers, and the means was expected to be to simplify the constructor.

What I did was what I always try to do, and what I think we’d all benefit from doing. I let the code participate in the planning. I reviewed it. It had messy little tags sticking out, people jamming values into fields and such. To make a better playing field, I improved those. As I did so, the constructors for the individual Flyers got better, and the main Flyer constructor got cleaned up and more orderly.

Now I’m feeling much less pressure to do the Pilot, though I still think it’s a good idea.

As it so often turns out, letting the code participate in its own design makes things better. I’m glad Kent Beck taught me to do that.

Stop by later, when I come back. We’ll do something else that may surprise both of us.