GitHub Decentralized Repo
GitHub Centralized Repo

I think we can simplify things a bit by adjusting the hierarchy of our space objects, now that a number of subclasses have been removed. Let’s see if I’m right. It could happen.

There are two interfaces at the top of the space object hierarchy:, ISpaceObject and InteractingSpaceObject:

interface InteractingSpaceObject {
    val subscriptions: Subscriptions
    fun callOther(other: InteractingSpaceObject, trans: Transaction)
}

interface ISpaceObject: InteractingSpaceObject {
    fun update(deltaTime: Double, trans: Transaction)
}

I have felt for some time that these are upside down, or something like that. I’ve made a couple of attempts to invert them, earlier on, but the code changes were too extensive to make it seem worthwhile. Now, however we are down to almost no implementors of either of these.

ISpaceObject is implemented by Asteroid, DeferredAction, Missile, Saucer, Score, Ship, and Splat. That’s everyone. The only additional implementor of InteractingSpaceObject is the ISpaceObject interface itself.

What is weird, or more weird, is that the classes all explicitly reference both interfaces in their definition. This means that they all implement all three of the required methods. However, not all of them actually use subscriptions and callOther: DeferredAction defines both as empty. That’s why I think the hierarchy is upside down. Score implements all three methods as empty: it is handled separately during applyChanges in SpaceObjectCollection (knownObjects).

For reasons that escape me, draw is a subscription. I think I probably just did that to fold it in with the rest. As long as subscriptions exist, that’s probably OK, but Splat only subscribes to draw. The others, Asteroid, Missile, Saucer, and Ship, use the full power of this fully operational deathstar the subscription capability.

I’d like to sort this out a bit. Since everyone now implements both these interfaces, it seems to me that we can merge them into one. Then I’d like to put a new interface on top, with no requirements, just enough to get everything to fit into Transaction and SpaceObjectCollection. That would simplify objects like Score and DeferredAction, which neither update nor interact.

I think IDEA’s Push Members Down or Pull Members Up could do this, but it looks to be going to change more than I have in mind. I’ll just move the abstract methods from InteractingSpaceObject down to ISpaceObject and see how that goes.

This gives me a moment’s concern, because the overrides for some of the objects don’t resolve. I need to change the interface they refer to. This is going to get weird, but I can always roll back. In fact, I do and try again:

interface ISpaceObject: InteractingSpaceObject {
    val subscriptions: Subscriptions
    fun callOther(other: ISpaceObject, trans: Transaction)
    fun update(deltaTime: Double, trans: Transaction)
}

Last time I didn’t change the callOther signature to refer to ISpaceObject. Try this.

Right. Now I have to edit all the override fun callOther implementations to use ISpaceObject as well. With that done, I’m green. Commit: move abstract methods from InteractingSpaceObject down to ISpaceObject.

Now the names of these interfaces are wrong. The hierarchy is InteractingSpaceObject > ISpaceObject, and it’s ISpaceObject that contains the interaction stuff. Let’s rename. It’ll take three steps:

  1. Rename InteractingSpaceObject to XXX;
  2. Rename ISpaceObject to InteractingSpaceObject
  3. Rename XXX to SpaceObject.

OK, here goes. Should be uneventful.

interface SpaceObject {
}

interface InteractingSpaceObject: SpaceObject {
    val subscriptions: Subscriptions
    fun callOther(other: InteractingSpaceObject, trans: Transaction)
    fun update(deltaTime: Double, trans: Transaction)
}

Test, just because I don’t really trust anyone. All green. Commit: Hierarchy is now SpaceObject>InteractingSpaceObject. No members in SpaceObject.

Now I’d like to remove the redundant declarations of both interfaces that occur. I’ll show the old examples as I do them:

class Asteroid(
    override var position: Point,
    val velocity: Velocity = U.randomVelocity(U.ASTEROID_SPEED),
    override val killRadius: Double = U.ASTEROID_KILL_RADIUS,
    private val splitCount: Int = 2
) : InteractingSpaceObject, SpaceObject, Collider {

class Missile(
    shooterPosition: Point,
    shooterHeading: Double = 0.0,
    shooterKillRadius: Double = U.SHIP_KILL_RADIUS,
    shooterVelocity: Velocity = Velocity.ZERO,
    val color: ColorRGBa = ColorRGBa.WHITE,
    val missileIsFromShip: Boolean = false
): InteractingSpaceObject, SpaceObject, Collider {

class Saucer : InteractingSpaceObject, SpaceObject, Collider {

class Score(val score: Int): InteractingSpaceObject {
	// added SpaceObject to this one!

class Ship(
    override var position: Point,
    val controls: Controls = Controls(),
    override val killRadius: Double = U.SHIP_KILL_RADIUS
) : InteractingSpaceObject, SpaceObject, Collider {

class Splat(
    var position: Point,
    var scale: Double = 1.0,
    var color: ColorRGBa = ColorRGBa.WHITE,
    val velocity: Velocity = Velocity.ZERO
) : InteractingSpaceObject, SpaceObject {

Let’s see if this still runs green. I think it should be just fine. Yes. Commit: remove some redundant references to SpaceObject in class definitions.

Now what I want to do is to make SpaceObjectCollection and Transaction expect SpaceObject, where they currently (probably) expect InteractingSpaceObject. That, if I am not mistaken, will let me simplify things a bit. So far, we’ve not accomplished much and I’ve been here for 50 minutes. (Takes me a while to write this stuff. The refactoring was mostly the click of a button.)

Transaction is rife with references to InteractingSpaceObject1. Replace them all with SpaceObject.

We are green. In Transaction, I just changed all the InteractingSpaceObject references to SpaceObject. In SpaceObjectCollection, things got a bit more tricky. In particular, I changed the forEach function, used in Game, to return only InteractingSpaceObjects:

    fun forEachInteracting(action: (InteractingSpaceObject)->Unit) = 
        spaceObjects.filterIsInstance<InteractingSpaceObject>().forEach(action)

That’s what Game wants, that’s what it gets. Another odd thing happened:

    fun allCollections(): List<MutableList<out SpaceObject>> {
        return listOf (spaceObjects, attackers, targets, deferredActions)
    }

I only use this function once:

    fun clear() {
        for ( coll in allCollections()) {
            coll.clear()
        }
    }

I don’t understand the out, but I’ll look it up when I get a moment. IDEA recommended it to me to make the code compile, so I accepted it.

In my tests, I had to add as DeferredAction a few times, to get them to compile. This was due to the removal of the update method from SpaceObject, so no real surprise.

And I have two tests commented out:

// Needs replacement?
    @Test
    fun `removeAll removes from all collections`() {
        val s = SpaceObjectCollection()
        val toRemove: MutableSet<InteractingSpaceObject> = mutableSetOf()
        for ( coll in s.allCollections()) {
            val toAdd = Asteroid(U.CENTER_OF_UNIVERSE)
            toRemove.add(toAdd)
            coll.add(toAdd)
        }
        s.removeAll(toRemove)
        for ( coll in s.allCollections()) {
            assertThat(coll).isEmpty()
        }
    }

// Score isn't in any collections
    @Test
    fun `removeAndFinalizeAll removes from all collections`() {
        val s = SpaceObjectCollection()
        val toRemove: MutableSet<InteractingSpaceObject> = mutableSetOf()
        for ( coll in s.allCollections()) {
            val toAdd = Score(666)
            toRemove.add(toAdd)
            coll.add(toAdd)
        }
        s.removeAndFinalizeAll(toRemove)
        for ( coll in s.allCollections()) {
            assertThat(coll).isEmpty()
        }
    }

I think the second one is no longer valid. The first one may be valid but since allColllections returns the deferred actions, the add won’t apply there any more. For reference, here’s the full SpaceObjectCollection:

class SpaceObjectCollection {
    var scoreKeeper = ScoreKeeper()
    val spaceObjects = mutableListOf<InteractingSpaceObject>()
    val attackers = mutableListOf<InteractingSpaceObject>()
    val targets = mutableListOf<InteractingSpaceObject>()
    val deferredActions = mutableListOf<DeferredAction>()
    // update function below if you add to these
    fun allCollections(): List<MutableList<out SpaceObject>> {
        return listOf (spaceObjects, attackers, targets, deferredActions)
    }

    fun add(spaceObject: SpaceObject) {
        when (spaceObject) {
            is Score -> scoreKeeper.addScore(spaceObject.score)
            is DeferredAction -> deferredActions.add(spaceObject)
            else -> addActualSpaceObjects(spaceObject as InteractingSpaceObject)
        }
    }

    private fun addActualSpaceObjects(spaceObject: InteractingSpaceObject) {
        spaceObjects.add(spaceObject)
        when (spaceObject) {
            is Missile -> attackers.add(spaceObject)
            is Asteroid -> targets.add(spaceObject)
            is Ship -> {
                attackers.add(spaceObject)
                targets.add(spaceObject)
            }
            is Saucer -> {
                attackers.add(spaceObject)
                targets.add(spaceObject)
            }
        }
    }

    fun addAll(newbies: Collection<SpaceObject>) {
        newbies.forEach{ add(it) }
    }

    fun any(predicate: (InteractingSpaceObject)-> Boolean): Boolean {
        return spaceObjects.any(predicate)
    }

    fun applyChanges(transaction: Transaction) = transaction.applyChanges(this)

    fun asteroidCount(): Int = targets.filterIsInstance<Asteroid>().size

    fun clear() {
        for ( coll in allCollections()) {
            coll.clear()
        }
    }

    fun forEachInteracting(action: (InteractingSpaceObject)->Unit) =
        spaceObjects.filterIsInstance<InteractingSpaceObject>().forEach(action)

    fun contains(obj:InteractingSpaceObject): Boolean {
        return spaceObjects.contains(obj)
    }

    fun pairsToCheck(): List<Pair<InteractingSpaceObject, InteractingSpaceObject>> {
        val pairs = mutableListOf<Pair<InteractingSpaceObject, InteractingSpaceObject>>()
        spaceObjects.indices.forEach { i ->
            spaceObjects.indices.minus(0..i).forEach { j ->
                pairs.add(spaceObjects[i] to spaceObjects[j])
            }
        }
        return pairs
    }

    fun performWithTransaction(action: (Transaction) -> Unit ) {
        val trans = Transaction()
        action(trans)
        applyChanges(trans)
    }

    fun removeAndFinalizeAll(moribund: Set<SpaceObject>) {
        moribund.forEach {
            if ( it is InteractingSpaceObject) addAll(it.subscriptions.finalize())
        }
        removeAll(moribund)
    }

    fun removeAll(moribund: Set<SpaceObject>) {
        spaceObjects.removeAll(moribund)
        attackers.removeAll(moribund)
        targets.removeAll(moribund)
        deferredActions.removeAll(moribund)
    }

    fun remove(spaceObject: SpaceObject) {
        removeAll(setOf(spaceObject))
    }

    fun saucerMissing(): Boolean {
        return targets.filterIsInstance<Saucer>().isEmpty()
    }

    fun shipIsPresent(): Boolean {
        return attackers.filterIsInstance<Ship>().isNotEmpty()
    }

    val size get() = spaceObjects.size
}

It’s a long one, at 106 lines. My friends will mock me if they find out. You’ll see that I changed most of the references to InteractingSpaceObject2 to SpaceObject, but there are a few other small changes that needed to be made.

We’re green and the game works perfectly. I’ll commit this and improve more next time. It’s time now for our Sunday ritual breakfast and watching of the Sunday Morning program.

Summary

IDEA did most of the hard work here, once I cut and pasted the abstracts down. The tests showed me the rest. This was actually a straightforward refactoring. I think the size of SpaceObjectCollection and its slightly more complicated refactoring is telling us something, but I’m not sure quite what that is. It does embody a fair bit of functionality: maybe there’s a way to split it up. We’ll see

We’ll take advantage of the new hierarchy next time. Today we created it and made it work.

See you next time!


  1. If I had known I was going to type that word so often I’d have called it Boo. ↩

  2. Darn, that word came up a lot. ↩