Kotlin 140
Sometimes you just know you have a good idea. Sometimes you’re even right. Is today one of those sometimes?
There’s a hand-off between the update and interaction phases of my special SpaceObjects, making them tricky to write and test. Often (at least twice) the update needs to skip a turn to be sure that the interaction has a result, or some other bumpety sort of thing. I have an idea that I believe will fix that up and make these special objects much simpler.
Let’s tell all the objects that we’re beginning an interaction cycle, and at the end of the cycle, tell them it’s over … and allow them to give us things to add and remove.
OK, that’s maybe one and a half ideas, or two, or two and a half. But the core notion is that if the objects can know interaction is beginning, they can init any counters or flags they need, and if they can return things when it’s all over, they can make decisions right there rather than wait for the next update.
I am quite sure that it’ll be better. Most objects will ignore these two new calls. Some will not.
I’m not going to TDD the basic calling: I can rely on my existing tests, and IDEA, to tell me that it isn’t working, and the tests for the individual objects, old and new, will make sure.
We’ll put the new methods in as defaults in the ISpaceObject interface, and override them as we improve existing objects and write new ones.
There are a few subjects that will come up along the way, and I think we’ll find at least one new object trying to be discovered. Let’s just do it. Here’s the life cycle code now:
fun cycle(drawer: Drawer, seconds: Double) {
val deltaTime = seconds - lastTime
lastTime = seconds
update(deltaTime)
processInteractions()
draw(drawer)
}
We want two new methods: beginInteractions and finishInteractions:
Pretty sure they go like this:
fun cycle(drawer: Drawer, seconds: Double) {
val deltaTime = seconds - lastTime
lastTime = seconds
update(deltaTime)
beginInteractions()
processInteractions()
finishInteractions()
draw(drawer)
}
The methods don’t exist. IDEA will help. I’ll fill them in. First the easy one:
private fun beginInteractions() {
knownObjects.forEach { it.beginInteraction() }
}
IDEA informs me that no one understands me. That’s what I’ve been telling you. Let’s put the method, with default, into ISpaceObject:
interface ISpaceObject ...
fun beginInteraction() {}
Tests should be green. They are. Can’t quite commit because of the TODO in the other method. I shouldn’t have put it in, perhaps. Anyway we can make it null for now. Commit: null beginInteraction in place.
Now at this moment, just a few minutes after we started, all the space objects have the ability to know that interactions are starting. None of them care.
The finishInteractions function is a bit more tricky, because we want to allow it to return things. I’m sure we want it to return things to be added, and nearly sure we want it also to return things to be removed. For now, and I promise to fix this, we’ll say that the individual calls to end can return a Pair containing things to be added and things to remove. I believe we can apply these changes as we go, because we are finished interacting and haven’t started updating, so no one is looking at the known objects. Therefore:
private fun finishInteractions() {
knownObjects.forEach {
val result: Pair<List<ISpaceObject>,MutableSet<ISpaceObject>> = {it.finishInteraction()}
knownObjects.addAll(result.first)
knownObjects.removeAll(result.second)
}
}
I am slightly surprised to note that the two elements of the pair require different collection types. We should sort that out, either using more generic collections, collections of our own invention, or just the same darn collection type in each case. Right now, those are the types that addAll and removeAll want, and we’ll go with it. The finish method is missing and we can default it in ISpaceObject.
IDEA offers to help but isn’t very good at it. I’ll add it:
fun finishInteraction(): Pair<List<ISpaceObject>, Set<ISpaceObject>> {
return Pair(emptyList(), emptySet())
}
I note that I’m returning Set here, not mutable. I plan to fix that in post.
First, change the type of the return:
private fun finishInteractions() {
knownObjects.forEach {
val result: Pair<List<ISpaceObject>,Set<ISpaceObject>> = {it.finishInteraction()}
knownObjects.addAll(result.first)
knownObjects.removeAll(result.second)
}
}
Then let’s go see why removeAll cares about mutable and change it not to:
fun removeAll(moribund: Set<ISpaceObject>): Boolean{
return spaceObjects.removeAll(moribund.toSet())
}
I think this ought to compile and run. But no, I get this message:
Type mismatch: inferred type is () -> Pair<List<ISpaceObject>, Set<ISpaceObject>> but Pair<List<ISpaceObject>, Set<ISpaceObject>> was expected
Referring to this line:
val result: Pair<List<ISpaceObject>,Set<ISpaceObject>> = {it.finishInteraction()}
Do I need a return in there? No, I need to remove the brackets. I don’t want to set result to a function, I want to call the function:
private fun finishInteractions() {
knownObjects.forEach {
val result: Pair<List<ISpaceObject>,Set<ISpaceObject>> = it.finishInteraction()
knownObjects.addAll(result.first)
knownObjects.removeAll(result.second)
}
}
This should run green. Commit: Game now calls beginInteraction and finishInteraction on all space objects, expects return of Pair(toAdd, toRemove) from the latter.
Perfect. Added a whole new convention, fully implemented. It does nothing … yet.
Now I promised myself that this would make some things easier. Let’s find some thing, and make it easier. I think it’ll help the ShipMonitor but only a fool would head in there with a new untried idea.
I think for a moment that WaveMaker might benefit, but it will not: we need another thing for it. What about WaveChecker:
class WaveChecker: ISpaceObject {
var firstTime = true
var lookingForAsteroid = false
override var elapsedTime = 0.0
override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
if ( lookingForAsteroid && other is SolidObject && other.isAsteroid)
lookingForAsteroid = false
return emptyList()
}
override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
return this.interactWith(other)
}
override fun update(deltaTime: Double): List<ISpaceObject> {
val result = mutableListOf<ISpaceObject>()
elapsedTime += deltaTime
if (elapsedTime > 1.0) {
if (firstTime) {
lookingForAsteroid = true
firstTime = false
}else if (lookingForAsteroid) {
elapsedTime = -5.0
firstTime = true
lookingForAsteroid = false
result.add(WaveMaker(4))
} else {
elapsedTime = 0.0
firstTime=true }
}
return result
}
}
This one is certainly worth improving if we can.
Let’s describe first what this is supposed to do, then explore how it does it.
Every second, check to see whether there are any asteroids. If there are not, create a WaveMaker, and wait 6 seconds before checking again.
The six-second delay is there because the WaveMaker delays for three seconds and only then creates the objects.
Now how does this thing accomplish that simple trick? With difficulty. We have a firstTime flag that starts true, and lookingForAsteroid that starts false. Elapsed time starts at zero.
When elapsedTime > 1 and firstTime is set, we set looking for asteroid true and firstTime false. That will cause the interaction loop to set lookingForAsteroid back to false if there are any. Therefore, at the end of interactions (and back in of the next update), if lookingForAsteroids is still true then we create a wave maker, set the looking flag back to false and firstTime back to true. And, if in the update lookingForAsteroids is still false, we just set first time to true and wait for the next tick.
I made this work yesterday, by brute force of intellect, burning the last few brain cells that I had left. Surely we can do this more simply now that we have this new capability.
I’m not sure I can TDD this. Let’s see if I can code it and then we’ll talk about tests. I propose a new implementation like this:
- In
update, incrementelapsedTime. That is all. - In beginInteractions, set
sawAsteroidto false. - In interact, if elaspedTime > 1.0 then
- if we see an asteroid, set
sawAsteroidto true
- if we see an asteroid, set
- In finishInteractions, if elapsedTime > 1.0 then
- if
sawAsteroidis false set elapsedTime to -5, return WaveMaker - else set elapsedTime to 0.0
- if
The check for elapsed time in interacting is redundant and a matter of efficiency. We’ll leave it out.
Let’s do this:
class WaveChecker: ISpaceObject {
var sawAsteroid = false
override var elapsedTime = 0.0
override fun beginInteraction() {
sawAsteroid = false
}
override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
if (other is SolidObject && other.isAsteroid)
sawAsteroid = true
return emptyList()
}
override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
return this.interactWith(other)
}
override fun finishInteraction(): Pair<List<ISpaceObject>, Set<ISpaceObject>> {
if ( elapsedTime > 1.0 ) {
elapsedTime = 0.0
if (!sawAsteroid) {
elapsedTime = -5.0
return Pair(listOf(WaveMaker(1)), emptySet())
}
}
return Pair(emptyList(), emptySet())
}
override fun update(deltaTime: Double): List<ISpaceObject> {
elapsedTime += deltaTime
return emptyList()
}
}
OK, I admit, I just coded that up, rather than somehow test driving it. I had the algorithm in mind and just wrote it. Even having just done it, I’m not seeing how TDD would have helped. But we’ll come to that.
It has more methods than before, and the same number of lines, but it is simpler. There’s only the one flag, and it only pertains to interactions, rather than the two flags that were shared between interactions and updates.
To test this, in the game, I had to comment out all three of the current WaveChecker tests, which were looking at the flags and therefore simply didn’t apply. Let’s review them now and see what we might do that will help us test this. I think it will also tell us how I might have test-driven it if I were smart enough.
Here are the tests as they were. They won’t even compile now, as they reference lookingForAsteroid.
class WaveCheckerTest {
@Test
fun `checker returns nothing on first update`() {
val ck = WaveChecker()
assertThat(ck.lookingForAsteroid).isEqualTo(false)
val toCreate = ck.update(0.1)
assertThat(toCreate).isEmpty()
}
@Test
fun `checker does nothing for one second, then counts asteroids`() {
val a = SolidObject.asteroid(U.randomPoint(), U.randomWelocity(1000.0))
val ck = WaveChecker()
assertThat(ck.lookingForAsteroid).describedAs("beginning").isEqualTo(false)
var toCreate = ck.update(0.51)
assertThat(ck.lookingForAsteroid).describedAs("after half a second").isEqualTo(false)
assertThat(toCreate).isEmpty()
toCreate = ck.update(0.51)
assertThat(toCreate).isEmpty()
assertThat(ck.lookingForAsteroid).describedAs("after 1 second timeout").isEqualTo(true)
assertThat(ck.elapsedTime).isEqualTo(1.02, within(0.1))
val toDestroy = ck.interactWith(a)
assertThat(toDestroy).isEmpty()
assertThat(ck.lookingForAsteroid).isEqualTo(false)
}
@Test
fun `checker creates WaveMaker and resets`() {
val ck = WaveChecker()
var toCreate = ck.update(1.1)
assertThat(toCreate).isEmpty()
assertThat(ck.lookingForAsteroid).describedAs("start looking").isEqualTo(true)
assertThat(ck.elapsedTime).describedAs("still ticking").isEqualTo(1.1)
// see no asteroids
toCreate = ck.update(0.1)
assertThat(toCreate[0]).isInstanceOf(WaveMaker::class.java)
assertThat(ck.elapsedTime).describedAs("long delay after triggering").isEqualTo(-5.0)
}
}
What would we like to test that would be like these?
- With elapsedTime < 1, the interaction calls return nothing.
- With elapsedTime > 1, and no asteroid sent through, the finish returns a waveMaker, and elapsedTime is set to -5
- With elapsedTime > 1, and an asteroid sent thru, finish returns empty and elapsedTime is set to zero.
@Test
fun `checker returns nothing when elapsed lt 1`() {
val ck = WaveChecker()
ck.update(0.5)
ck.beginInteraction()
val resultPair = ck.finishInteraction()
assertThat(ck.elapsedTime).isEqualTo(0.5)
val toCreate = ck.update(0.1)
assertThat(toCreate).isEmpty() // always is, don't check again
}
Let’s try that. Green. Now the second test.
@Test
fun `returns WaveMaker when elapsed gt 1 and no asteroid scanned`() {
val ck = WaveChecker()
ck.update(1.1)
ck.beginInteraction()
val resultPair = ck.finishInteraction()
assertThat(resultPair.first[0]).isInstanceOf(WaveMaker::class.java)
assertThat(ck.elapsedTime).isEqualTo(-5.0)
}
And the third:
@Test
fun `returns empty when elapsed gt 1 and an asteroid IS scanned`() {
val a = SolidObject.asteroid(U.randomPoint(), U.randomWelocity(1000.0))
val ck = WaveChecker()
ck.update(1.1)
ck.beginInteraction()
ck.interactWith(a)
val resultPair = ck.finishInteraction()
assertThat(resultPair.first).isEmpty()
assertThat(resultPair.second).isEmpty()
assertThat(ck.elapsedTime).isEqualTo(0.0)
}
Green. We can commit. But I literally laughed out loud when I noticed the velocity call setting up the asteroid. All this time it has been randomWelocity like I was channeling Ensign Chekov or something. Let’s do rename that.
Committed. Here’s the new WaveChecker in toto.
class WaveChecker: ISpaceObject {
var sawAsteroid = false
override var elapsedTime = 0.0
override fun beginInteraction() {
sawAsteroid = false
}
override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
if (other is SolidObject && other.isAsteroid)
sawAsteroid = true
return emptyList()
}
override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
return this.interactWith(other)
}
override fun finishInteraction(): Pair<List<ISpaceObject>, Set<ISpaceObject>> {
if ( elapsedTime > 1.0 ) {
elapsedTime = 0.0
if (!sawAsteroid) {
elapsedTime = -5.0
return Pair(listOf(WaveMaker(1)), emptySet())
}
}
return Pair(emptyList(), emptySet())
}
override fun update(deltaTime: Double): List<ISpaceObject> {
elapsedTime += deltaTime
return emptyList()
}
}
It’s Sunday, and brekkers is right around the corner, so let’s sum up.
Summary
Oh, I forgot to mention one error that I fixed, in the finish code:
private fun finishInteractions() {
val bufferAdds = mutableListOf<ISpaceObject>()
val bufferRemoves = mutableSetOf<ISpaceObject>()
knownObjects.forEach {
val result: Pair<List<ISpaceObject>,Set<ISpaceObject>> = it.finishInteraction()
bufferAdds.addAll(result.first)
bufferRemoves.removeAll(result.second)
}
knownObjects.addAll(bufferAdds)
knownObjects.removeAll(bufferRemoves)
}
I had to buffer the results, because you can’t go modifying the knownObjects collection while iterating it. Kotlin simply cannot cope. The need to buffer is a bit irritating and we can try something different perhaps tomorrow. Work needs to be done there anyway: The return of the Pair of list / set is definitely weird. I think we’ll want an object of our own, perhaps Transaction instead of Pair, and it seems to me that we should standardize on a particular collection type. My guess is that it should be Set, but we’ll dig into that further. Lists are probably more efficient, if one were to care.
I’m sure there is more that we will want to do to improve this new scheme, but I think it has already clearly paid off. The new tests demonstrate the greater simplicity even more than the code, and the code itself is simpler. I predict that we’ll be able to make the ShipMonitor much simpler than it currently is now that the concept has been proven.
One other thing that I keep wanting to look at: it seems to me that if we were to settle on a scheme, most of the objects, perhaps all of them, could default one of the interactWith methods. I’m not certain of that. I think we know that all the SolidObjects want always to do interactWithOther as their interactWith function. It’s all a bit odd, anyway, and perhaps could be better. It just seems that we do much the same thing every time, so it can probably be simplified.
- Big Question
-
Well maybe not that big, but the past few sessions, I’ve not been able to see quite how to TDD the objects. I’ve seen — more accurately nearly seen — the algorithm, but how to test did not spring to mind. I’ve learned that that’s a risky way to go, and it seems to be increasing. Must keep this in mind and try to work out what’s causing the resistance to tests.
-
Relatedly, I was slow to commit changes, winding up at the end with a number of changes that I committed separately. I appreciate IDEA’s help in doing that: you can select each file and look at its diffs, and see whether you want to commit it separately, revert out spurious prints, or whatever. Very nice. But I think if I were on my game, my commits would all be very small and I wouldn’t use that capability much if at all.
Overall, I’m pleased that, this once, a seemingly good idea seems to actually be good. I love it when that happens. See you next time!