Kotlin 204 - A small cleanup
GitHub Decentralized Repo
GitHub Centralized Repo
I had an idea the other day, something I should probably have done a while back, so with a little time this afternoon, I’ll try it now.
We have this bit in SpaceObjectCollection (knownObjects
):
fun add(spaceObject: ISpaceObject) {
if ( spaceObject is Score ) {
scoreKeeper.addScore(spaceObject.score)
return
}
if ( spaceObject is DeferredAction) {
deferredActions.add(spaceObject)
return
}
spaceObjects.add(spaceObject)
if (spaceObject is Missile) attackers.add(spaceObject)
if (spaceObject is Ship) {
attackers.add(spaceObject)
targets.add(spaceObject)
}
if (spaceObject is Saucer) {
attackers.add(spaceObject)
targets.add(spaceObject)
}
if (spaceObject is Asteroid) targets.add(spaceObject)
}
It seems to me that we can implement add for each specific class to good effect. We’ll try it. Like this, for example:
fun add(score: Score) {
scoreKeeper.addScore(score.score)
}
fun add(spaceObject: ISpaceObject) {
if ( spaceObject is DeferredAction) {
deferredActions.add(spaceObject)
return
}
spaceObjects.add(spaceObject)
if (spaceObject is Missile) attackers.add(spaceObject)
if (spaceObject is Ship) {
attackers.add(spaceObject)
targets.add(spaceObject)
}
if (spaceObject is Saucer) {
attackers.add(spaceObject)
targets.add(spaceObject)
}
if (spaceObject is Asteroid) targets.add(spaceObject)
}
Test. Oh, hell, doesn’t work. Type checking isn’t dynamic in Kotlin. I prefer dynamic. Oh well, bummer. Roll back. Can we do this some other way?
I guess if we wanted to do this, we’d have to do something in Transaction, because when we add to a Transaction we generally know what we’re adding. Let’s look.
class Transaction
fun add(spaceObject: ISpaceObject) {
adds.add(spaceObject)
}
We could, in principle, break out each class into separate collections here, then apply each collection separately. There aren’t that many now. Implementors are Asteroid, DeferredAction, Missile, Saucer, Score, Ship, and Splat. Seven. Of those, I think only Asteroids are ever added more than one per transaction.
I don’t think that’s worth doing. Let’s see if we can improve the add method some other way.
Can we convert that code to when
in any reasonable way?
fun add(spaceObject: ISpaceObject) {
if ( spaceObject is Score ) {
scoreKeeper.addScore(spaceObject.score)
return
}
if ( spaceObject is DeferredAction) {
deferredActions.add(spaceObject)
return
}
addActualSpaceObjects(spaceObject)
}
private fun addActualSpaceObjects(spaceObject: ISpaceObject) {
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)
}
}
}
That’s perhaps somewhat better. Let’s now try this:
fun add(spaceObject: ISpaceObject) {
if ( spaceObject is Score ) {
scoreKeeper.addScore(spaceObject.score)
return
}
else if ( spaceObject is DeferredAction) {
deferredActions.add(spaceObject)
return
}
addActualSpaceObjects(spaceObject)
}
Adding that else allows IDEA to suggest a when. Let’s try that:
fun add(spaceObject: ISpaceObject) {
when (spaceObject) {
is Score -> {
scoreKeeper.addScore(spaceObject.score)
return
}
is DeferredAction -> {
deferredActions.add(spaceObject)
return
}
else -> addActualSpaceObjects(spaceObject)
}
}
private fun addActualSpaceObjects(spaceObject: ISpaceObject) {
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)
}
}
}
OK, not exactly wonderful but perhaps a bit more readable. But can’t I remove those return statements?
Yes, that gives me this:
fun add(spaceObject: ISpaceObject) {
when (spaceObject) {
is Score -> {
scoreKeeper.addScore(spaceObject.score)
}
is DeferredAction -> {
deferredActions.add(spaceObject)
}
else -> addActualSpaceObjects(spaceObject)
}
}
And then I can one-line those:
fun add(spaceObject: ISpaceObject) {
when (spaceObject) {
is Score -> scoreKeeper.addScore(spaceObject.score)
is DeferredAction -> deferredActions.add(spaceObject)
else -> addActualSpaceObjects(spaceObject)
}
}
private fun addActualSpaceObjects(spaceObject: ISpaceObject) {
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)
}
}
}
That’s better than it was, I think. Tests are green. This was a machine refactoring except for adding the else. I can commit and will: Refactor add
for clarity.
But I did notice a glitch when playing. The first time the Saucer appears, if it was previously on the screen during attract mode, it starts out at the position it had when you put in the quarter.
I think it needs a wakeup call, which is what resets it to the edge, or else we should create a new one when we init the game. Let’s do that.
private fun createInitialObjects(
trans: Transaction,
shipCount: Int,
controls: Controls
) {
cancelAllOneShots()
trans.clear()
saucer = Saucer()
scoreKeeper = ScoreKeeper(shipCount)
knownObjects.scoreKeeper = scoreKeeper
val shipPosition = U.CENTER_OF_UNIVERSE
ship = Ship(shipPosition, controls)
}
Had to make saucer a var instead of a val. Could reset it. Let’s do this:
class Saucer
init {
initialize()
}
fun initialize() {
direction = -1.0
wakeUp()
}
class Game
private fun createInitialObjects(
trans: Transaction,
shipCount: Int,
controls: Controls
) {
cancelAllOneShots()
trans.clear()
saucer.initialize()
scoreKeeper = ScoreKeeper(shipCount)
knownObjects.scoreKeeper = scoreKeeper
val shipPosition = U.CENTER_OF_UNIVERSE
ship = Ship(shipPosition, controls)
}
That’s better, I think. The fewer mutable variables we have, the better.
Curiously but not surprisingly, now Kotlin doesn’t realize that direction
is initialized, since it isn’t clever enough to trace down through my code. So I have to init direction
to -1.0 up top. No biggie.
Works. Green. I’m not well-pleased but I am somewhat pleased. Commit: Fixed defect where Saucer sometimes appeared mid-screen.
Summary
Well, I just came in to do a little cleanup, and I did that. I was mistakenly thinking that Kotlin was smart enough to pick the right add
function dynamically but no such luck. My mistake, too much duck typing in my past.
Anyway we cleaned up a bit of stuff and found a somewhat obscure bug that had been around for a while.
Nothing to sneeze at, as we used to say in the old days.
See you next time!