GitHub Repo

I think something needs to be done about the growing mess in the flyers. Let’s take a look and see what we see. I’m trying to honor GeePaw’s concerns … and yet … and yet, I go a different way than I anticipated, with better results.

GeePaw Hill and I differ in a number of regards, despite the incredible good looks and charm that we hold in common, and our very similar experienced-based affection for the techniques that I here tend to call “Agile”. One of our differences is that he prefers not to use the “Agile” term, and considering how badly it has been besmirched, I can understand that. But we do agree on very many things.

Warning
In what follows, I’ll do my best to describe Hill’s concerns and views fairly. Know, however, that I do not know his mind and surely have only remembered a fraction of what he has told me, and understood even less. So to get his views, ask him. Still, I’m doing my best here.

Perhaps the largest difference in our practice is that GeePaw tends to have a design in mind, and to be writing a program that meets that design … or, more accurately … a good design that is much like the design he had in mind. He wouldn’t argue that he has the whole design in mind, or that what he has in mind is quite correct. But he tends to have a shape in mind when he builds a program.

Generally speaking, I myself do not have so much of a shape in mind. I used to work more like GeePaw does, and I certainly know how, having studied all manner of design techniques over the years. I work as I do, concerned mostly with local ideas, for two reasons. First, I enjoy giving the code a shape and hearing it say “no, not like that”, and listening to what I see in the shape I’ve made. Second, my approach invariably leads to code that is more messy than it might be, and because I think most of us work with messy code in our professional lives, I enjoy showing how to spot the mess and how to improve things. Third1, I tend to be more careless than I should be, often charging ahead and doing something unsupported by tests or even by an existing need. It’s good for me, and for others, I hope, to see what happens.

Another difference between GeePaw’s way and my way is that he avoids object hierarchy with concrete behavior in the superclasses. I don’t know if we was frightened by a superclass at some point or what, but he is very firm about this belief. In this context, however, he has commented:2 “NB I am widely considered to be a crank.”

I, having been raised by Jesuits and subsequently a band of Smalltalkers, have commonly used concrete methods in my hierarchies and have only crashed and burned some of the time. Other times I have prospered, or at least that’s the way I remember things.

Last night …

Our Zoom Ensemble / Friday Geeks Night Out did look at the code of Asteroids, and while no conclusions were drawn, we found ourselves sort of leaning in the direction of pluggable behavior. I think it was Hill himself who suggested that maybe there were a few different interfaces merged in these objects, and that identifying those might be useful. Whoever said it, I didn’t completely understand it, though I’m sort of getting the drift this morning.

That brings us to today.

In the present Asteroids program, I made a sort of anti-design decision. Since my normal approach would have included little objects for ship, missile, asteroid, and so on, I decided not to do that, instead putting all that behavior into a single class, whose current name is Flyer, because they all fly about in space. So the Flyer class is getting a bit messy, and it’s easy to see that it’s only going to get worse if we don’t do something about it.

A random idea has led to an interesting overall design — and to a bit more mess.

Today we have an interface IFlyer, which seems broad, particularly in member variables, but also somewhat in functions, and we have a few implementors of the interface. We’ll see shortly that there are common elements among the implementors, and significant differences as well. I would be inclined to move some of the sameness up in an object hierarchy, and to move differences down.

Now in fairness to Hill, there is one thing that can immediately give trouble doing this. Suppose there are two aspect of behavior to your objects. Perhaps they can move in a couple of ways, and they can interact in a couple of ways. Well, if all four combinations are possible, next thing you know you’re building a superclass for each combination of behaviors and OMG if there are three groups, four, next thing you know you’ve got N factorial (!) combinations to deal with. That would be bad.

Let’s look at the code and see if we can classify what’s going on, maybe get some vague concepts to start moving apart. We’ll start with the Flyer, representing all the objects that, well, fly.

However: there are really two kinds of objects in the universe, under an interface IFlyer. The other objects, which I’ll refer to here as “special”, don’t move around, but they do interact with all the other objects. The specials handle scoring, re-spawning ships, and probably other matters in the future.

We’ll start by looking at Flyer, the class for all the moving objects, the ones you see on the screen.

First, the methods, because maybe they’re easier:

  • accelerate
  • asSplit
  • asTwin
  • collisionDamageWith
  • collisionDamageWithOther
  • draw
  • finalize
  • getScore
  • move
  • tick
  • toString
  • turnBy
  • update

Let me try to group these according to some notion of purpose.

Moving in Space
move moves the object according to its velocity. All Flyers move. The special object in the mix do not. Flyers have a velocity, Others default it to zero but execute the move method anyway.

accelerate adjusts the object’s velocity if it accelerates. only the ship does this.

turnBy adjusts the object’s heading. Only the ship does this.

Interactions
collisionDamageWith is called for each pair of objects. a.collisionDamageWith(b) unconditionally dispatches b.collisionDamageWithOther.a. This double dispatch ensures that the special object in the mix always have control during their interactions.

collisionDamageWithObject handles the actual interaction of flying objects, called by collisionDamageWith. Real flyers can collide and generally destroy each other when they do. The special objects use the collision logic to perform their special actions, such as adjusting the game score.

finalize is called when a collision interaction has destroyed an object. This gives it the chance to register score or go out in a blaze of glory. Blaze of glory is not yet implemented. Finalize is a result of an interaction via the collision methods, which can return objects to be destroyed. The finalize method can return new objects to be added into the mix.

asSplit and asTwin create an asteroid half the size of the current one, and duplicate it, for return via finalize, to implement the big asteroids splitting into two smaller ones.

getScore is called when a flying object is finalized, and adds a Score special object to the mix, bearing the score to be added for destroying that object. Only asteroids return non-zero scores at present, but we might add a Score for shooting down the Saucer or its missiles. The Score object is consumed upon interaction with the ScoreKeeper special object.

Drawing
draw holds the common code for drawing a flyer. The specific drawing is done by an instance of a class implementing interface FlyerView, such as ShipView or AsteroidView. Those classes just have one method, also named draw.
Updating
update is the generic method called on all objects to allow them to update their situation. For Flyers, it ticks the clock, uses the controls to allow the object to update its heading or acceleration (or to fire a missile), then calls move. It returns a list of new objects to be added to the mix. The list will either contain a missile or it will not, but in principle multiple things could be added here. Special objects have their own update method, usually just returning an empty list and taking no other action.

tick updates an internal clock that each flying object maintains. Flying objects have a lifetime and are automatically destroyed by the LifetimeClock special object when they exceed their lifetime. Only missiles have a finite lifetime.

Utility
toString provides an intelligible string for printing flying objects.

What might we do?

I can see several “tools” for untangling this mess, including

Flyer Subclasses
We could create separate subclasses classes for Asteroid, Missile, Ship, and so on. There would be some common code and some unique code. We might better see how to deal with the commonalities and differences. I am mostly resisting this approach, just to see how other ideas might help.
Strategy Classes
As we have for drawing, we could plug in small classes to handle an object’s special behavior on colliding, finalizing, updating, and so on. We have one case of this now, could try a few more.
Strategy Lambdas
Instead of defining small classes to do the things we want, we could just pass in functions to do the specialized work. We have no cases of this. It seems somewhat like the “modern” way of doing things, passing functions around rather than objects. We might try it just to learn. And it might turn out to be quite good.

What shall we do?

I think I’d like to find a place to try the Lambda idea, just to try it on for size. I think a good case is in finalize: … and getScore:

    override fun finalize(): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (splitCount >= 1) { // type check by any other name
            val meSplit = asSplit()
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

    private fun getScore(): Score {
        val score = when (killRadius) {
            500.0 -> 20
            250.0 -> 50
            125.0 -> 100
            else -> 0
        }
        return Score(score)
    }

The bulk of this code is about asteroids. All the other flyers return nothing when finalizing.

Let’s first inline the getScore .. oh and that tells me something:

    override fun finalize(): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = Score(
            when (killRadius) {
                500.0 -> 20
                250.0 -> 50
                125.0 -> 100
                else -> 0
            }
        )
        if (score.score > 0) objectsToAdd.add(score)
        if (splitCount >= 1) { // type check by any other name
            val meSplit = asSplit()
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

Honestly, that’s way too messy to package into a lambda. Something that complicated is a job for an object. Undo that, we’ll save the clever lambda idea for now, but we’ll introduce a Finalizer object.

Let’s see if I can TDD this object. I might be willing to extract a function and plug it back in but this is going to require an Interface and a couple of concrete classes.

AsteroidFinalizer

    @Test
    fun `asteroid finalizer`() {
        val fin = AsteroidFinalizer()
        val asteroid = Flyer.asteroid(Point.ZERO)
        val splits = fin.finalize(asteroid)
        assertThat(splits.size).isEqualTo(3) // split guys and a score
    }

This is more than enough to get us going. IDEA wants to create the class for me. I’ll allow it. Then it wants to create the method finalize.

class AsteroidFinalizer {
    fun finalize(asteroid: Flyer): List<IFlyer> {
        return emptyList()
    }
}

That should fail nicely, with nothing in the net. Well, no, it won’t compile because asteroids have a required velocity.

    @Test
    fun `asteroid finalizer`() {
        val fin = AsteroidFinalizer()
        val asteroid = Flyer.asteroid(Point.ZERO, Velocity.ZERO)
        val splits = fin.finalize(asteroid)
        assertThat(splits.size).isEqualTo(3) // split guys and a score
    }

Careful readers are wondering what Point and Velocity even are. Last night the Zoom crew inspired me to typealias Vector2 to some words more meaningful. Now test again.

expected: 3
 but was: 0

Yep. Now let’s move over the code from the Flyer:

class AsteroidFinalizer {
    fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = getScore(asteroid)
        if (score.score > 0 ) objectsToAdd.add(score)
        if (asteroid.splitCount >= 1) { // type check by any other name
            val meSplit = asteroid.asSplit()
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

    private fun getScore(asteroid: Flyer): Score {
        val score = when (asteroid.killRadius) {
            500.0 -> 20
            250.0 -> 50
            125.0 -> 100
            else -> 0
        }
        return Score(score)
    }
}

I expect the test to run. It does. And I’ve done this:

class Flyer ...
    override fun finalize(): List<IFlyer> {
        return AsteroidFinalizer().finalize(this)
    }

That should leave everything working as it was. And all my tests are green. I can’t resist trying the game. I should have never implemented the graphical bit. It works well, and I waste some time racking up 4160 points. That should equal, um 820 + 1650 + 32*100. Yep. Perfect score for one round. Of course I have an infinite number of ships and there’s no charge for using one. Anyway, it works.

Now let’s give the Flyer a member variable and a default to a simpler finalize, one that does nothing.

class Flyer( ...
    var finalizer = DefaultFinalizer()
    ...

    override fun finalize(): List<IFlyer> {
        return finalizer.finalize(this)
    }

class DefaultFinalizer() {
    fun finalize(flyer: Flyer): List<IFlyer> {
        return emptyList()
    }
}

As written, this should break a lot of tests. And it does. We need these things:

  1. Extract an Interface for the finalizers.
  2. Declare that on the member variable.
  3. Put the right kind in when we create an asteroid.
interface IFinalizer {
    fun finalize(flyer: Flyer): List<IFlyer>
}

class AsteroidFinalizer: IFinalizer {
    override fun finalize(asteroid: Flyer): List<IFlyer> {
    ...


class DefaultFinalizer() : IFinalizer {
    override fun finalize(flyer: Flyer): List<IFlyer> {
    ...

And in Flyer:

    var finalizer: IFinalizer = DefaultFinalizer()

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

I’m not proud of the also but it should inject the right finalizer without having to mess with the already horrid Flyer constructor. I expect this to work. It does. All the tests pass and the game works.

Let’s move the creation of the twins over into the new Asteroid finalizer, to clean up Flyer class a bit.

    private fun Flyer.asSplit(): Flyer {
        splitCount -= 1
        killRadius /= 2.0
        velocity = velocity.rotate(Math.random() * 360.0)
        return this
    }

    private fun Flyer.asTwin() = Flyer.asteroid(
        pos = position,
        vel = velocity.rotate(Math.random() * 360.0),
        killRad = killRadius,
        splitCt = splitCount
    )

Should still be just fine. We are green. Commit: Flyer finalization deferred to finalizer member, implementors of IFinalizer interface.

Is that righteous, implementing asSplit and asTwin as extension methods, inside AsteroidFinalizer? It seems to me that it is, and it’s rather nice as they are private inside it. It is, after all, the only one who uses it.

We might do the same with getScore, which is now:

    private fun getScore(asteroid: Flyer): Score {
        val score = when (asteroid.killRadius) {
            500.0 -> 20
            250.0 -> 50
            125.0 -> 100
            else -> 0
        }
        return Score(score)
    }

I think I will do that, for symmetry if nothing else:

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = asteroid.getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (asteroid.splitCount >= 1) { // type check by any other name
            val meSplit = asteroid.asSplit()
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

    private fun Flyer.getScore(): Score {
        val score = when (killRadius) {
            500.0 -> 20
            250.0 -> 50
            125.0 -> 100
            else -> 0
        }
        return Score(score)
    }

Test. Green. Commit: make getScore private Flyer extension in AsteroidFinalizer.

Let’s review all out changes and see what we think.

interface IFinalizer {
    fun finalize(flyer: Flyer): List<IFlyer>
}

class DefaultFinalizer() : IFinalizer {
    override fun finalize(flyer: Flyer): List<IFlyer> {
        return emptyList()
    }
}

class AsteroidFinalizer: IFinalizer {
    private fun Flyer.asSplit(): Flyer {
        splitCount -= 1
        killRadius /= 2.0
        velocity = velocity.rotate(Math.random() * 360.0)
        return this
    }

    private fun Flyer.asTwin() = Flyer.asteroid(
        pos = position,
        vel = velocity.rotate(Math.random() * 360.0),
        killRad = killRadius,
        splitCt = splitCount
    )

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = asteroid.getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (asteroid.splitCount >= 1) { // type check by any other name
            val meSplit = asteroid.asSplit()
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

    private fun Flyer.getScore(): Score {
        val score = when (killRadius) {
            500.0 -> 20
            250.0 -> 50
            125.0 -> 100
            else -> 0
        }
        return Score(score)
    }
}

class Flyer(
    override var position: Point,
    override var velocity: Velocity,
    override var killRadius: Double,
    var splitCount: Int = 0,
    override val ignoreCollisions: 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

    fun accelerate(deltaV: Acceleration) {
        velocity = (velocity + deltaV).limitedToLightSpeed()
    }

    override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
        return other.collisionDamageWithOther(this)
    }

    override fun collisionDamageWithOther(other: IFlyer): List<IFlyer> {
        if ( this === other) return emptyList()
        if ( this.ignoreCollisions && other.ignoreCollisions) return emptyList()
        val dist = position.distanceTo(other.position)
        val allowed = killRadius + other.killRadius
        return if (dist < allowed) listOf(this,other) else emptyList()
    }

    override fun draw(drawer: Drawer) {
        val center = Point(drawer.width/2.0, drawer.height/2.0)
        drawer.fill = ColorRGBa.MEDIUM_SLATE_BLUE
        drawer.translate(position)
        view.draw(this, drawer)
    }

    override fun finalize(): List<IFlyer> {
        return finalizer.finalize(this)
    }

    override fun move(deltaTime: Double) {
        position = (position + velocity * deltaTime).cap()
    }

    fun tick(deltaTime: Double) {
        elapsedTime += deltaTime
    }

    override fun toString(): String {
        return "Flyer $position ($killRadius)"
    }

    fun turnBy(degrees:Double) {
        heading += degrees
    }

    override fun update(deltaTime: Double): List<Flyer> {
        tick(deltaTime)
        val result: MutableList<Flyer> = mutableListOf()
        val additions = controls.control(this, deltaTime)
        result.addAll(additions)
        move(deltaTime)
        return result
    }

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

I dumped more code than just what we changed, so we can get a bigger picture, not just of what we’ve done, but where we stand now.

Finalizer
I think that change made a definite improvement. We removed three Flyer methods outright, asSplit, asTwin, and getScore, and added them as private extensions inside AsteroidFinalizer. We moved all the complexity of Flyer.finalize, with a simple finalizer for most flyers, and the more complex one just applied to asteroids … and broken out into a separate class for just that purpose.

I had wanted to try using a lambda, “just” passing in a function to do the job, but clearly we had way too much behavior to embed in a single function, so the interface-class approach made more sense. We’ll save the lambda trick for another time … although now that we are using classes for these purposes, we might find it best to stick with the pattern. We’ll see.

Overall
The Flyer still has a lot of stuff in it, with no fewer than eleven member variables, of which seven appear in the class’s interface IFlyer. We’d like to limit that.

The method for dealing with collision seems more complex than it might be, and the update method seems a bit longish as well. Note that they are five and six lines, respectively. Yes, I really do consider five or six lines to be a “long” method. Really. Not the 150 or 500 that I’ve seen in Certain People’s Code. Five or Six.

There are some variables in there that I think we clearly need for all flyers, notable position and velocity. Those could be bound together, perhaps, in to a Dynamics object. Maybe we could even plug acceleration and heading in there. Dynamics would have to interact with drawing, which needs to know the heading, which would add a bit of complexity. There is also the question of interaction with Controls. Perhaps Controls is one kind of Dynamics object? This notion is pretty fresh in my mind, and I’ve not given it a lot of thought, but it seems like an interesting idea.

Summary

I feel that there’s no question that what we have here is a better design, though it is far from a great design. I can still see places where breaking out flyers by type would make things easier. But we’ll find out as we go forward. i think I’ll try to keep away from that particular breakout, as long as it make sense, to get a solid feeling about this different kind of structure. The pieces are smaller than in the flyer subclass kinds of solutions I’ve done before.

It’s interesting, at least to me, and, I hope, to you, both of my readers. See you next time!


  1. I was wrong about the “two”. Happens so often. ↩

  2. Private communication. ↩