GitHub Decentralized Repo
GitHub Centralized Repo

It’s time to address centralizing creation of the Ship, including hyperspace. This is one of the more complicated bits in the game. (Ron keeps a spike. Shall we withhold his treats?)

In the decentralized version, there are two cooperating objects that take turns in the mix, ShipChecker and ShipMaker. The first, ShipChecker, is usually present. Its sole purpose is to detect when the ship has gone missing, which it does via the standard interactions:

        beforeInteractions = { missingShip = true },
        interactWithShip = { _, _ -> missingShip = false },
        afterInteractions = { trans ->
            if ( missingShip && (ship.inHyperspace || scoreKeeper.takeShip())) {
                trans.add(ShipMaker(ship, scoreKeeper))
                trans.remove(this)
            }
        }

It assumes that the ship is missing; records the happy surprise if it turns out to be there, and if the ship is in fact missing (and is either in hyperspace or there is another ship available) the ShipChecker adds a ShipMaker to the mix and removes itself.

The same ship is reused, so it is passed to the maker, as is the ScoreKeeper. Note that the ScoreKeeper is used in deciding whether to rez a new ship, so it is passed back and forth between checker and maker, so that the soon to be created new checker will have access to it.

ShipMaker delays emergence of a new ship if the old one is killed, in two ways. First, it always waits for a discreet interval SHIP_MAKER_DELAY, to give the player a chance to get ready. Second, it will not rez the new ship if there are asteroids near the emergence point, or if the saucer is on the screen.

If the ship is in hyperspace, however, it is immediately replaced. There is further rigmarole around that, because going to hyperspace can kill you. We’ll look at that below.

Let’s look at ShipMaker in a few phases.

class ShipMaker(val ship: Ship, val scoreKeeper: ScoreKeeper = ScoreKeeper()) : ISpaceObject, InteractingSpaceObject {
    var safeToEmerge = true
    var asteroidTally = 0
    private var elapsedTime = 0.0

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
    }

We initialize a bit, and on update, we increment our elapsedTime. (ShipMaker predates the invention of the OneShot and DeferredAction, and it never seemed like a good idea to convert it to use them.)

    override val subscriptions = Subscriptions (
        beforeInteractions = {
            safeToEmerge = true
            asteroidTally = 0
        },
        interactWithMissile = { _, _ -> safeToEmerge = false },
        interactWithAsteroid = { asteroid, _ ->
            asteroidTally += 1
            safeToEmerge = isAnythingInTheWay(asteroid)
        },
        interactWithSaucer = { _, _ ->
            safeToEmerge = false
        },
        afterInteractions = { trans->
            if (ship.inHyperspace || elapsedTime > U.SHIP_MAKER_DELAY && safeToEmerge) {
                replaceTheShip(trans)
            }
        }
    )

During interaction, we assume that it will be safe to emerge. Then we count any asteroids that exist, because hyperspace emergence uses that count to decide whether you make it out safely. And we check to see if the asteroid is in the way, and if it is, or if we see a saucer at all, we set that it is not safe to emerge.

Finally, after interactions, we replace the ship under two conditions, either

  1. Immediately, if we’re in hyperspace. We don’t check for safety, we just go for it.
  2. After the discreet delay, if it is safe to emerge.

Replacing the ship looks like this:

    private fun replaceTheShip(trans: Transaction) {
        trans.remove(this)
        if (!ship.inHyperspace || hyperspaceOK()) {
            trans.add(ship)
            ship.dropIn()
        } else {
            ship.inHyperspace = false
            trans.add(Splat(ship))
            ship.finalizeObject() // dead again
        }
        trans.add(ShipChecker(ship, scoreKeeper))
    }

We remove the ShipMaker (this). If the ship is not in hyperspace, or hyperspaceOK is true, we add the ship and tell it to do its “drop in” animation. Otherwise, we are in hyperspace and we are not OK. We mark the ship as not in hyperspace (because it is going to die), we add an explosion Splat, and we finalize the ship.

Finally, having removed the ShipMaker, we add a new ShipChecker back in and everything continues.

For the record, Ship.finalize does this:

    fun finalizeObject(): List<ISpaceObject> {
        if ( inHyperspace ) {
            position = U.randomInsidePoint()
        } else {
            position = U.CENTER_OF_UNIVERSE
            velocity = Velocity.ZERO
            heading = 0.0
        }
        return emptyList()
    }

Essentially this sets the ship to emerge at the center of the screen, not moving, aimed to the right, if we’re not in hyperspace, which, in the case just above, we are not. The finalize doesn’t add any objects, so it returns an empty list.

Complicated, right?

Well, a little bit, but in fact it’s pretty simple in concept. The ShipChecker notices that the ship is gone and rezzes a ShipMaker, which checks the situation and either rezzes the ship or an explosion, or does nothing at all if there are no more ships to be had.

I guess if we have to say this and that or this or that, the object is complicated.

In the decentralized game, thinking about it now, I could imagine that ShipChecker might determine why the ship is gone and create either an object that takes a ship from inventory and rezzes it at center, or another object that manages hyperspace emergence. Each of those would be simpler and more straightforward than Maker is.

We might even manage hyperspace more directly, closer to the control operation, so that the ShipMaker only deals with ships from inventory.

But we’re not refactoring the decentralized version, we’re here to do what is necessary to centralized this capability.

The question is how. We’ll do some planning.

Planning

I’ve been thinking about this a bit and it seems to me that having hyperspace entangled with ship replacement is messy, and we shouldn’t do it that way again. So my starting plan is to do them separately:

  1. Hyperspace should be managed directly at the time the control is triggered. We’ll pick a random location to emerge. We’ll check for safety and if it isn’t safe, create an explosion at the emergence point and remove the ship. If it is safe, we’ll place the ship at that point, triggering the drop in animation. The ship will only disappear from the mix if it is destroyed.

  2. If the ship is destroyed, it will disappear from the mix. The game will check for ship missing, similarly to how it checks now for asteroids and saucers, and if there is a ship available will trigger a OneShot to set a flag, createShip. If there is no ship available, nothing is done. (The ScoreKeeper will display Game Over.)

  3. When the createShip flag is seen to be on, the game will check for saucer gone and a clear central area and if things are good, it will replace the ship.

That seems to me to be roughly what has to happen. What I do not see is how to do this in small steps that keep the game working, and that keep us using the new code. We could hide the new code behind a “feature flag”, which we’d set for testing and reset for production. And we can certainly TDD some of the code that we need, and simply not use it until it’s all ready.

But what would be even better would be if we could change things bit by bit, truly incrementally moving from the decentralized to the centralized.

What if …

Here are some loosely-related things that might allow for small steps:

  • Move hyperspace handling out first, simplifying ShipMaker accordingly. That will make it smaller and easier to replace.

  • Change Game to check explicitly for the ship and to create a ShipMaker but don’t put it in the mix. Instead, tell it directly from Game what to do. When it reports done, remove it. Game probably has two inner states, checking and making.

But what if …

Here’s another idea.

  1. Move hyperspace out and simplify ShipMaker. Game still works.

  2. Pull ShipChecker logic into Game and have it create a simplified ShipMaker in the mix. Game will need to know not to create more ShipMakers. Probably use a OneShot. Game still works.

  3. Instead of putting ShipMaker into the mix, create it as a helper object for Game. Give it access to game so that it can call back to get the information it needs about saucer presence and asteroid proximity. It can fetch all the asteroids and check them in a loop. Game still works.

  4. Refactor as seems to be needed. Game still works.

Decision

I think a first step is shaping up here, which will be to remove hyperspace from concern by doing it explicitly as a control operation.

Let’s try a spike.

In controls we have this:

    fun control(ship: Ship, deltaTime: Double, trans: Transaction) {
        if (hyperspace) {
            hyperspace = false
            ship.enterHyperspace(trans)
        }
        turn(ship, deltaTime)
        accelerate(ship, deltaTime)
        trans.addAll(fire(ship))
    }

What does ship do?

    fun enterHyperspace(trans: Transaction) {
        inHyperspace = true
        trans.remove(this)
    }

It sets a flag and removes itself, trusting ShipCheck and ShipMaker to do the right thing. Let’s change that.

Hyperspace sets a random location, possibly destroys the ship, and triggers drop in animation. Let’s just do the first and last, like this:

    fun enterHyperspace(trans: Transaction) {
        position = U.randomInsidePoint()
        dropIn()
    }

If I’m not mistaken, that should make hyperspace work instantly and perfectly every time. Of course you might rez too close to something to escape it. This change breaks a couple of hyperspace tests. No surprise there. In the Game, hyperspace looks the same except that you always survive.

Let’s enhance our spike. This is going so well, we might keep it1.

    fun enterHyperspace(trans: Transaction) {
        if (hyperspaceOK()) {
            position = U.randomInsidePoint()
            dropIn()
        } else {
            trans.remove(this)
            trans.add(Splat(this))
        }
    }

Could it be this easy?

We may need to set position to center, for safety:

    fun enterHyperspace(trans: Transaction) {
        if (hyperspaceOK()) {
            position = U.randomInsidePoint()
            dropIn()
        } else {
            position = U.CENTER_OF_UNIVERSE
            trans.remove(this)
            trans.add(Splat(this))
        }
    }

And we need hyperspaceOK. I’ll bring that over from ShipMaker, with some mods.

Ah, there’s the rub:

    private fun hyperspaceOK(): Boolean = !hyperspaceFailure(Random.nextInt(0, 63), asteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidTally: Int): Boolean
            = random0thru62 >= (asteroidTally + 44)

We need to know how many asteroids there are. Ship doesn’t know that, and doesn’t know anyone who does know it. Controls only knows the ship. Game knows knownObjects and it can return the asteroid count, but we have no access to game.

Aside
One of the nice things about the decentralized version is that there are few explicit connections between objects, but they get a chance to interact with anyone they care to. Here, the fact that objects don’t know each other is a problem for us.

What can we do about this?

  • We could create a global variable, probably in U, that always has the asteroid count.
  • Controls could know the Game, ask it for the count, and pass it to the ship
  • We could create Ship to know the Game.

I think we’re less exposed if we create the U variable. Let’s try that. We’re getting closer to wanting to throw our spike away, but we’ll see what we think.

object U
    var AsteroidTally = 0 // evil global value

class Game
    fun cycle(elapsedSeconds: Double, drawer: Drawer? = null) {
        val deltaTime = elapsedSeconds - lastTime
        lastTime = elapsedSeconds
        tick(deltaTime)
        beginInteractions()
        processInteractions()
        finishInteractions()
        U.AsteroidTally = knownObjects.asteroidCount()
        createNewWaveIfNeeded()
        createSaucerIfNeeded()
        drawer?.let { draw(drawer) }
    }

class Ship
    private fun hyperspaceOK(): Boolean = !hyperspaceFailure(Random.nextInt(0, 63), U.AsteroidTally)

I think this now includes hyperspace failure as it was before. I’ll test in game a bit.

There’s one bug: the splat always occurs at the center. Code in the wrong order:

    fun enterHyperspace(trans: Transaction) {
        if (hyperspaceOK()) {
            position = U.randomInsidePoint()
            dropIn()
        } else {
            trans.add(Splat(this))
            position = U.CENTER_OF_UNIVERSE
            trans.remove(this)
        }
    }

That will fix that. Still not right. Now it explodes where it was. We need the random point:

    fun enterHyperspace(trans: Transaction) {
        position = U.randomInsidePoint()
        if (hyperspaceOK()) {
            dropIn()
        } else {
            trans.add(Splat(this))
            position = U.CENTER_OF_UNIVERSE
            trans.remove(this)
        }
    }

This is temporal coupling, by the way. These things have to be done in the right order.

This is working perfectly. Let’s check those failing tests and see if we can recover them.

    @Test
    fun `does not debit ScoreKeeper if ship is in hyperspace`() {

    @Test
    fun `ship randomizes position on hyperspace entry`() {

The first of these doesn’t apply any more, because we don’t destroy the ship on hyperspace entry any more.

The second one we might be able to recast, but the first could just go away.

The original test:

    @Test
    fun `ship randomizes position on hyperspace entry`() {
        val ship = Ship(U.CENTER_OF_UNIVERSE)
        val trans = Transaction()
        ship.enterHyperspace(trans)
        assertThat(trans.firstRemove()).isEqualTo(ship)
        ship.finalizeObject()
        assertThat(ship.position).isNotEqualTo(U.CENTER_OF_UNIVERSE)
    }

We don’t need the penultimate and antepenultimate lines. However, if we do this:

    @Test
    fun `ship randomizes position on hyperspace entry`() {
        val ship = Ship(U.CENTER_OF_UNIVERSE)
        val trans = Transaction()
        ship.enterHyperspace(trans)
        assertThat(ship.position).isNotEqualTo(U.CENTER_OF_UNIVERSE)
    }

This test could fail if the odds are against the ship. However, if we set asteroidTally large enough, we can ensure that it’ll never fail.

    @Test
    fun `ship randomizes position on hyperspace entry`() {
        val ship = Ship(U.CENTER_OF_UNIVERSE)
        val trans = Transaction()
        U.AsteroidTally  = 100 // hyperspace never fails
        ship.enterHyperspace(trans)
        assertThat(ship.position).isNotEqualTo(U.CENTER_OF_UNIVERSE)
    }

Remove the other test as unneeded. We are green.

Now we have a decision to make: Do we keep this, or not? If we keep it, we can simplify ShipMaker, making tomorrow’s job easier. Let’s review what we’ve done:

object U
    var AsteroidTally = 0 // evil global value

class Game
    fun cycle(elapsedSeconds: Double, drawer: Drawer? = null) {
        val deltaTime = elapsedSeconds - lastTime
        lastTime = elapsedSeconds
        tick(deltaTime)
        beginInteractions()
        processInteractions()
        finishInteractions()
        U.AsteroidTally = knownObjects.asteroidCount()
        createNewWaveIfNeeded()
        createSaucerIfNeeded()
        drawer?.let { draw(drawer) }
    }

    private fun createNewWaveIfNeeded() {
        if ( U.AsteroidTally == 0 ) {  // changed to use the tally
            val trans = Transaction()
            waveOneShot.execute(trans)
            knownObjects.applyChanges(trans)
        }
    }

class Ship
    fun enterHyperspace(trans: Transaction) {
        position = U.randomInsidePoint()
        if (hyperspaceOK()) {
            dropIn()
        } else {
            trans.add(Splat(this))
            position = U.CENTER_OF_UNIVERSE
            trans.remove(this)
        }
    }

    private fun hyperspaceOK(): Boolean = !hyperspaceFailure(Random.nextInt(0, 63), U.AsteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidTally: Int): Boolean
            = random0thru62 >= (asteroidTally + 44)

I don’t see that things could be much better than this. The biggest design issue is the need to have the asteroidTally and that’s about as minimally invasive as it could be.

I’m going to commit this, but I’ll review the decision in Summary and will take questions from the audience. Commit: hyperspace now done by Ship without destroying itself in the normal case.

Now we can remove the hyperspace stuff from ShipMaker. Here’s the before:

class ShipMaker(val ship: Ship, val scoreKeeper: ScoreKeeper = ScoreKeeper()) : ISpaceObject, InteractingSpaceObject {
    var safeToEmerge = true
    var asteroidTally = 0
    private var elapsedTime = 0.0

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
    }

    override val subscriptions = Subscriptions (
        beforeInteractions = {
            safeToEmerge = true
            asteroidTally = 0
        },
        interactWithMissile = { _, _ -> safeToEmerge = false },
        interactWithAsteroid = { asteroid, _ ->
            asteroidTally += 1
            safeToEmerge = isAnythingInTheWay(asteroid)
        },
        interactWithSaucer = { _, _ ->
            safeToEmerge = false
        },
        afterInteractions = { trans->
            if (ship.inHyperspace || elapsedTime > U.SHIP_MAKER_DELAY && safeToEmerge) {
                replaceTheShip(trans)
            }
        }
    )

    private fun isAnythingInTheWay(collider: Collider) = safeToEmerge && !tooClose(collider)

    override fun callOther(other: InteractingSpaceObject, trans: Transaction) =
        other.subscriptions.interactWithShipMaker(this, trans)

    private fun tooClose(collider: Collider): Boolean {
        return ship.position.distanceTo(collider.position) < U.SAFE_SHIP_DISTANCE
    }

    private fun replaceTheShip(trans: Transaction) {
        trans.remove(this)
        if (!ship.inHyperspace || hyperspaceOK()) {
            trans.add(ship)
            ship.dropIn()
        } else {
            ship.inHyperspace = false
            trans.add(Splat(ship))
            ship.finalizeObject() // dead again
        }
        trans.add(ShipChecker(ship, scoreKeeper))
    }

    private fun hyperspaceOK(): Boolean = !hyperspaceFailure(Random.nextInt(0, 63), asteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidTally: Int): Boolean
            = random0thru62 >= (asteroidTally + 44)
}

We can remove those last two functions as not needed.

In replaceTheShip we are never in hyperspace, so only the first branch can happen.

    private fun replaceTheShip(trans: Transaction) {
        trans.remove(this)
        trans.add(ShipChecker(ship, scoreKeeper))
        trans.add(ship)
        ship.dropIn()
    }

The afterInteractions doesn’t need to check hyperspace:

        afterInteractions = { trans->
            if (elapsedTime > U.SHIP_MAKER_DELAY && safeToEmerge) {
                replaceTheShip(trans)
            }
        }

There’s no real need to count the asteroids any more.

    override val subscriptions = Subscriptions (
        beforeInteractions = { safeToEmerge = true },
        interactWithMissile = { _, _ -> safeToEmerge = false },
        interactWithAsteroid = { asteroid, _ -> safeToEmerge = isAnythingInTheWay(asteroid) },
        interactWithSaucer = { _, _ -> safeToEmerge = false },
        afterInteractions = { trans->
            if (elapsedTime > U.SHIP_MAKER_DELAY && safeToEmerge) {
                replaceTheShip(trans)
            }
        }
    )

This is going to break some tests that use the asteroidTally. We’ll fix them.

The Ship will has that inHyperspace flag. Let’s expunge that. Ship has one large case:

    fun finalizeObject(): List<ISpaceObject> {
        if ( inHyperspace ) {
            position = U.randomInsidePoint()
        } else {
            position = U.CENTER_OF_UNIVERSE
            velocity = Velocity.ZERO
            heading = 0.0
        }
        return emptyList()
    }

We’re never in hyperspace now, so

    fun finalizeObject(): List<ISpaceObject> {
        position = U.CENTER_OF_UNIVERSE
        velocity = Velocity.ZERO
        heading = 0.0
        return emptyList()
    }

In ShipChecker:

        afterInteractions = { trans ->
            if ( missingShip && (ship.inHyperspace || scoreKeeper.takeShip())) {
                trans.add(ShipMaker(ship, scoreKeeper))
                trans.remove(this)
            }
        }

That becomes this:

        afterInteractions = { trans ->
            if ( missingShip && scoreKeeper.takeShip()) {
                trans.add(ShipMaker(ship, scoreKeeper))
                trans.remove(this)
            }
        }

I think that’s all. Now some tests surely break.

Some set asteroidTally to force a condition in ShipMaker but I think they’ll want to be deleted. And there’s this test:

    @Test
    fun `hyperspace failure checks`() {
        val ignoredShip = Ship(U.CENTER_OF_UNIVERSE)
        val hyper = ShipMaker(ignoredShip)
        assertThat(hyper.hyperspaceFailure(62, 19)).describedAs("roll 62 19 asteroids").isEqualTo(false)
        assertThat(hyper.hyperspaceFailure(62, 18)).describedAs("roll 62 18 asteroids").isEqualTo(true)
        assertThat(hyper.hyperspaceFailure(45, 0)).describedAs("roll 45 0 asteroids").isEqualTo(true)
        assertThat(hyper.hyperspaceFailure(44, 0)).describedAs("roll 44 0 asteroids").isEqualTo(true)
        assertThat(hyper.hyperspaceFailure(43, 0)).describedAs("roll 43 0 asteroids").isEqualTo(false)
    }

This is checking the odds. I think we can send that to the Ship.

    @Test
    fun `hyperspace failure checks`() {
        val ship = Ship(U.CENTER_OF_UNIVERSE)
        assertThat(ship.hyperspaceFailure(62, 19)).describedAs("roll 62 19 asteroids").isEqualTo(false)
        assertThat(ship.hyperspaceFailure(62, 18)).describedAs("roll 62 18 asteroids").isEqualTo(true)
        assertThat(ship.hyperspaceFailure(45, 0)).describedAs("roll 45 0 asteroids").isEqualTo(true)
        assertThat(ship.hyperspaceFailure(44, 0)).describedAs("roll 44 0 asteroids").isEqualTo(true)
        assertThat(ship.hyperspaceFailure(43, 0)).describedAs("roll 43 0 asteroids").isEqualTo(false)
    }

Yes that works fine. Run all tests. We are green. Commit: Remove all hyperspace logic from ShipChecker and ShipMaker. Remove inHyperspace flag from Ship. Adjust / remove tests.

I think we’re done here. Let’s take a final look at ShipMaker and then sum up.

class ShipMaker(val ship: Ship, val scoreKeeper: ScoreKeeper = ScoreKeeper()) : ISpaceObject, InteractingSpaceObject {
    var safeToEmerge = true
    private var elapsedTime = 0.0

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
    }

    override fun callOther(other: InteractingSpaceObject, trans: Transaction) =
        other.subscriptions.interactWithShipMaker(this, trans)

    override val subscriptions = Subscriptions (
        beforeInteractions = { safeToEmerge = true },
        interactWithMissile = { _, _ -> safeToEmerge = false },
        interactWithAsteroid = { asteroid, _ -> safeToEmerge = isAnythingInTheWay(asteroid) },
        interactWithSaucer = { _, _ -> safeToEmerge = false },
        afterInteractions = { trans->
            if (elapsedTime > U.SHIP_MAKER_DELAY && safeToEmerge) {
                replaceTheShip(trans)
            }
        }
    )

    private fun isAnythingInTheWay(collider: Collider) = safeToEmerge && !tooClose(collider)

    private fun tooClose(collider: Collider): Boolean {
        return ship.position.distanceTo(collider.position) < U.SAFE_SHIP_DISTANCE
    }

    private fun replaceTheShip(trans: Transaction) {
        trans.remove(this)
        trans.add(ShipChecker(ship, scoreKeeper))
        trans.add(ship)
        ship.dropIn()
    }
}

Summary

Shorter and simpler. I think this implementation of hyperspace is better than what we have in the decentralized version … and it would work just fine there. Combining hyperspace into the ShipMaker logic seemed like a good idea at the time, but the current approach of just repositioning and then possibly self-destructing is more direct and more clear, and it simplifies ShipMaker substantially and ShipChecker is simplified a bit as well.

The most questionable bit, I think, is the live U.AsteroidTally global. We have to provide some connection between the ship and the asteroid count, in order to determine hyperspace failure. As the game is designed, we have no connection between controls and the game, so a connection can’t be provided that way. We could create the ship knowing the game, but then we have a circular reference that we wouldn’t like.

At this moment I don’t see a simpler solution. That’s why I accepted the spike. But we should talk about that further.

Is this OK?

Isn’t there a rule that we always delete spiked code? There surely is one somewhere. Am I therefore a bad person? Perhaps, but that’s a weak reason.

I think the biggest fault I would lay at my door was that I started making my changes without writing a new test. But the original change was just these few lines:

~~~kotlin
    fun enterHyperspace(trans: Transaction) {
        position = U.randomInsidePoint()
        dropIn()
    }

Instead of setting flag and destroying the ship, I just moved it and triggered the drop in. Except for the failure, that perfectly implemented hyperspace. It seemed clear to me that there was nothing simpler than that, and so after that, we just followed our nose, moving the hyperspaceOK functions over, fixing tests, refining code, and so on.

And this wasn’t a spike for something new. It was a spike to implement a refactoring: keep hyperspace functioning the same and change the implementation. Far less risky than saving a spike of some new thing. And our tests did fail and tell us things, adding confidence that we were on the right track.

I think this morning’s code is righteous and I see no advantage to doing it over.

Could I be wrong? Yes. Even if this time I “got away with it”, isn’t there a chance that sometimes when I keep a spike it is a bad idea? Yes, there is a chance. It probably even happens.

But what harm can come from keeping a spike that would be better thrown away?

It might not actually work
Well, yes. Unless our tests are particularly robust. If it’s a new feature, and we’re spiking, we probably have little or no test support. Here, we were moving the functionality, not creating it anew.
It might be poor code and hard to maintain
Um, yes. In this case, the code is simple, simpler than it was. Objection overruled.
It might make future changes harder
OK, it might. That will happen, in particular, if we spike something in by grafting in an if statement or a temporary call. This was not the case. The change was unconditional and it seems to me that it’s quite solid.
But you’re just rationalizing
Possibly. But I’m thinking harder about it than a random code jockey who slams in an if statement and pushes the code. I did it in small steps and checked along the way.
You’re still rationalizing
I am. But my money says that this code is righteous. If it turns out it isn’t, we’ll revisit the question.
You’d better hope that there isn’t a bug in there.
Damn betcha. If there is, I’ll be the first to admit it and we can all mock me.

For now, I kept a spike and I’m not sorry. We’ve made it easier to do further steps in getting rid of the ShipChecker and ShipMaker.

Stop by next time and see how we go after them!


  1. Yes, I know you’re supposed to throw a spike away. I don’t always do that. Sometimes I get something right enough to keep it. I’ll discuss in Summary. Don’t try this at home. Or do, I’m not the boss of you. ↩