GitHub Decentralized Repo
GitHub Centralized Repo

I’ll take another run at finalize. I expect to succeed this time. I could be wrong.

I think first, I’ll move the current finalization for Ship and Saucer to initialization. Let’s start with ship:

    fun finalize(trans: Transaction) {
        position = U.CENTER_OF_UNIVERSE
        velocity = Velocity.ZERO
        heading = 0.0
    }

Let’s extract a method and call it setToHome:

    fun finalize(trans: Transaction) {
        setToHome()
    }

    fun setToHome() {
        position = U.CENTER_OF_UNIVERSE
        velocity = Velocity.ZERO
        heading = 0.0
    }

Let’s remove the call from here and move it into the DeferredAction that starts the ship:

    private val shipOneShot = OneShot(U.SHIP_MAKER_DELAY, { canShipEmerge() }) {
       if ( scoreKeeper.takeShip() ) {
           ship.setToHome()
           it.add(ship)
       }
    }

Test to see who will be disappointed. Some may be. This one fails:

    @Test
    fun `ship goes to center on collision death`() {
        val ship = Ship(U.randomPoint())
        val trans = Transaction()
        ship.collision(trans)
        assertThat(trans.firstRemove()).isEqualTo(ship)
        ship.finalize(Transaction())
        assertThat(ship.position).isEqualTo(U.CENTER_OF_UNIVERSE)
    }

Interesting. This is no longer our design. I think we need to extract a new method:

    fun startShipAtHome(trans: Transaction) {
        ship.setToHome()
        trans.add(ship)
    }

Rename and adjust the test to check game. I’m slightly irritated that this isn’t a ship test. I make a Game test that I’m not proud of:

    @Test
    fun `ship goes to center when game starts it`() {
        val game = Game()
        game.insertQuarter(Controls())
        val trans = Transaction()
        game.cycle(0.1)
        game.cycle(3.1)
        val ship = game.knownObjects.ships.first()
        ship.position = Point(100.0, 100.0)
        ship.collision(trans)
        assertThat(trans.firstRemove()).isEqualTo(ship)
        game.knownObjects.applyChanges(trans)
        assertThat(ship.position).isNotEqualTo(U.CENTER_OF_UNIVERSE)
        game.cycle(3.2)
        game.cycle(6.2)
        val shipAgain = game.knownObjects.ships.first()
        assertThat(shipAgain.position).isEqualTo(U.CENTER_OF_UNIVERSE)
    }

It runs green, showing that the ship gets reset to center. Ugly, though. We are green. Commit: Ship set to center of universe when added by oneShot. Finalize is empty.

Now we can remove Ship’s reference to finalize and the method.

    override val subscriptions = Subscriptions(
        interactWithAsteroid = { asteroid, trans -> checkCollision(asteroid, trans) },
        interactWithSaucer = { saucer, trans -> checkCollision(saucer, trans) },
        interactWithMissile = { missile, trans -> checkCollision(missile, trans) },
        draw = this::draw,
        finalize = this::finalize
    )

    fun finalize(trans: Transaction) {
    }

Still green. Commit: Ship no longer requires finalize.

OK, that was fairly easy, but the difficulty with that test suggests that we need something better. I think probably game should be sending a message to ship, one that we could test, but this will hold water for now.

Let’s do the Asteroid change. I think it should be easy.

    private fun finalize(trans: Transaction) {
        if (splitCount >= 1) {
            trans.add(asSplit(this))
            trans.add(asSplit(this))
        }
    }

Extract splitIfPossible:

    private fun finalize(trans: Transaction) {
        splitIfPossible(trans)
    }

    private fun splitIfPossible(trans: Transaction) {
        if (splitCount >= 1) {
            trans.add(asSplit(this))
            trans.add(asSplit(this))
        }
    }

Test. Green. Commit: extract splitIfPossible method, used in finalize.

Move the call from finalize to where the asteroid dies. I think we have a method for that?

    private fun dieIfColliding(missile: Missile, trans: Transaction) {
        if (Collision(missile).hit(this)) {
            trans.remove(this)
            trans.add(Splat(this))
        }
    }

Yes. We’ll just do the split here:

    private fun dieIfColliding(missile: Missile, trans: Transaction) {
        if (Collision(missile).hit(this)) {
            trans.remove(this)
            trans.add(Splat(this))
            splitIfPossible(trans)
        }
    }

I expect some tests may fail. We’ll see. Four do. That seems unfair. We’ll see what they are:

    @Test
    fun `asteroid splits on finalize`() {
        val full = Asteroid(
            position = Point.ZERO,
            velocity = Velocity.ZERO
        )
        val radius = full.killRadius
        val trans = Transaction()
        full.subscriptions.finalize(trans)
        val halfSize = trans.adds
        assertThat(halfSize.size).isEqualTo(2) // two asteroids and no score
        val half = halfSize.last()
        assertThat((half as Asteroid).killRadius).describedAs("half").isEqualTo(radius/2.0)
        val trans2 = Transaction()
        half.subscriptions.finalize(trans2)
        val quarterSize = trans2.adds
        assertThat(quarterSize.size).isEqualTo(2)
        val quarter = quarterSize.last()
        assertThat((quarter as Asteroid).killRadius).describedAs("quarter").isEqualTo(radius/4.0)
        val trans3 = Transaction()
        quarter.subscriptions.finalize(trans3)
        val eighthSize = trans3.adds
        assertThat(eighthSize.size).describedAs("should not split third time").isEqualTo(0)
    }

I think we need another extract:

    private fun dieIfColliding(missile: Missile, trans: Transaction) {
        if (Collision(missile).hit(this)) {
            dieDuetoCollision(trans)
        }
    }

    fun dieDuetoCollision(trans: Transaction) {
        trans.remove(this)
        trans.add(Splat(this))
        splitIfPossible(trans)
    }

Now I can use that in the test above:

    @Test
    fun `asteroid splits on collision death`() {
        val full = Asteroid(
            position = Point.ZERO,
            velocity = Velocity.ZERO
        )
        val radius = full.killRadius
        val trans = Transaction()
        full.dieDuetoCollision(trans)
        val halfAdds = trans.adds
        assertThat(halfAdds.size).isEqualTo(3) // two asteroids and a splat
        val half = halfAdds.last()
        assertThat((half as Asteroid).killRadius).describedAs("half").isEqualTo(radius/2.0)
        val trans2 = Transaction()
        half.dieDuetoCollision(trans2)
        val quarterAdds = trans2.adds
        assertThat(quarterAdds.size).isEqualTo(3)
        val quarter = quarterAdds.last()
        assertThat((quarter as Asteroid).killRadius).describedAs("quarter").isEqualTo(radius/4.0)
        val trans3 = Transaction()
        quarter.dieDuetoCollision(trans3)
        val eighthAdds = trans3.adds
        assertThat(eighthAdds.size).describedAs("should not split third time").isEqualTo(1) // just splat
    }

I hate these long story tests. But it’s green. What’s next?

The next one just needed subscriptions.finalize replaced with dieDueToCollision. I’ll spare you the code.

This one fails and so does my fixed version:

    @Test
    fun `colliding ship and asteroid splits asteroid, loses ship`() {
        val game = Game()
        val asteroid = Asteroid(Vector2(1000.0, 1000.0))
        val ship = Ship(
            position = Vector2(1000.0, 1000.0)
        )
        game.add(asteroid)
        game.add(ship)
        assertThat(game.knownObjects.size).isEqualTo(2)
        assertThat(ship).isIn(game.knownObjects.spaceObjects())
        game.processInteractions()
        assertThat(ship).isNotIn(game.knownObjects.spaceObjects())
        assertThat(game.knownObjects.asteroidCount()).isEqualTo(2)
    }

I don’t find two asteroids … and it’s not calling the die function. Ah. it’s an actual bug:

    override val subscriptions = Subscriptions(
        interactWithMissile = { missile, trans -> dieIfColliding(missile, trans) },
        interactWithShip = { ship, trans -> if (Collision(ship).hit(this)) trans.remove(this) },
        interactWithSaucer = { saucer, trans -> if (Collision(saucer).hit(this)) trans.remove(this) },
        draw = this::draw,
        finalize = this::finalize
    )

All those folks really need to be doing dieIfColliding, it seems to me. We’ll need class-specific ones:

    private fun dieIfColliding(missile: Missile, trans: Transaction) {
        if (Collision(missile).hit(this)) {
            dieDuetoCollision(trans)
        }
    }

    private fun dieIfColliding(ship: Ship, trans: Transaction) {
        if (Collision(ship).hit(this)) {
            dieDuetoCollision(trans)
        }
    }

    private fun dieIfColliding(saucer: Saucer, trans: Transaction) {
        if (Collision(saucer).hit(this)) {
            dieDuetoCollision(trans)
        }
    }

Now I think we can do this:

    override val subscriptions = Subscriptions(
        interactWithMissile = { missile, trans -> dieIfColliding(missile, trans) },
        interactWithShip = { ship, trans -> dieIfColliding(ship, trans) },
        interactWithSaucer = { saucer, trans -> dieIfColliding(saucer, trans) },
        draw = this::draw,
        finalize = this::finalize
    )

Test. All Green. Remove finalize. Green. Commit: asteroid split moved inside dieIfColliding, out of finalize. Finalize removed.

So, this is good. I think we’ll save the saucer for next time, this was just an afternoon romp.

Summary

After some practice this morning, removing finalize from Ship and Asteroid has gone smoothly. Once we get all uses removed, we can remove it from the Subscriptions object and it’ll be gone gone gone.

Again we see that small steps can add up. Will we prefer the final design? That remains to be seen. There will be more duplication than I prefer, I’m sure of that. But whether we can get rid of it, I’m not sure. There may be a way, once everything is in place. Or we may find that we don’t mind the way it is.

Join me next time, for another thrilling episode of The Incremental Refactoring Ranger.