Kotlin 217 - Renaming
GitHub Decentralized Repo
GitHub Centralized Repo
Let’s rename all the
interactWithThisAndThat
tointeract
. I think that will reduce unnecessary noise. We’ll see.
It’s pretty straightforward. I just look in Interactions and rename from there.
In less than five minutes, Interactions is changed, the tests are green. Interactions looks like this now:
class Interaction(
private val missiles: List<Missile>,
private val ships: List<Ship>,
private val saucers: List<Saucer>,
private val asteroids: List<Asteroid>,
private val trans: Transaction
) {
init {
missilesVsMissileShipSaucerAsteroids()
shipVsSaucerAsteroids()
saucerVsAsteroids()
}
private fun saucerVsAsteroids() {
saucers.forEach { saucer->
asteroids.forEach { asteroid ->
saucer.interact(asteroid, trans)
asteroid.interact(saucer, trans)
}
}
}
private fun shipVsSaucerAsteroids() {
ships.forEach { ship ->
saucers.forEach { saucer ->
ship.interact(saucer, trans)
saucer.interact(ship, trans)
}
asteroids.forEach { asteroid ->
ship.interact(asteroid, trans)
asteroid.interact(ship, trans)
}
}
}
private fun missilesVsMissileShipSaucerAsteroids() {
missiles.forEach { missile ->
missiles.forEach { other ->
if (other != missile ) {
missile.interact(other, trans)
other.interact(missile, trans)
}
}
ships.forEach { ship ->
ship.interact(missile, trans)
missile.interact(ship, trans)
}
saucers.forEach { saucer ->
saucer.interact(missile, trans)
missile.interact(saucer, trans)
}
asteroids.forEach { asteroid ->
asteroid.interact(missile, trans)
missile.interact(asteroid, trans)
}
}
}
}
Commit: rename all interactWithXXX
to interact
.
In Saucer, for example, all the implementations of interact
are different:
fun interact(ship: Ship, trans: Transaction) {
sawShip = true
shipFuturePosition = ship.position + ship.velocity * 1.5
checkCollision(ship, trans)
}
fun interact(asteroid: Asteroid, trans: Transaction) {
checkCollision(asteroid, trans)
}
fun interact(missile: Missile, trans: Transaction) {
if (missile == currentMissile) missileReady = false
checkCollision(missile, trans)
}
It seems to me that naming all the interaction methods interact
is better. The differences are clear, since the first argument’s name and type is right there in front of us.
In Asteroids, we have this:
fun interact(missile: Missile, trans: Transaction) {
if (Collision(missile).hit(this)) {
dieDueToCollision(trans)
}
}
fun interact(ship: Ship, trans: Transaction) {
if (Collision(ship).hit(this)) {
dieDueToCollision(trans)
}
}
fun interact(saucer: Saucer, trans: Transaction) {
if (Collision(saucer).hit(this)) {
dieDueToCollision(trans)
}
}
That compares to this, in Ship:
fun interact(saucer: Saucer, trans: Transaction) {
checkCollision(saucer, trans)
}
fun interact(asteroid: Asteroid, trans: Transaction) {
checkCollision(asteroid, trans)
}
fun interact(missile: Missile, trans: Transaction) {
checkCollision(missile, trans)
}
private fun checkCollision(other: Collider, trans: Transaction) {
if (weAreCollidingWith(other)) {
collision(trans)
}
}
private fun weAreCollidingWith(other: Collider): Boolean = Collision(other).hit(this)
These two should be more the same, and they both use Collision. Let’s try an extract in Asteroid.
fun interact(missile: Missile, trans: Transaction) {
checkCollision(missile, trans)
}
private fun checkCollision(missile: Missile, trans: Transaction) {
if (Collision(missile).hit(this)) {
dieDueToCollision(trans)
}
}
Now if I change the signature on that checkCollision
:
We’re still green. I can do this:
fun interact(missile: Missile, trans: Transaction) {
checkCollision(missile, trans)
}
fun interact(ship: Ship, trans: Transaction) {
checkCollision(ship, trans)
}
fun interact(saucer: Saucer, trans: Transaction) {
checkCollision(saucer, trans)
}
private fun checkCollision(other: Collider, trans: Transaction) {
if (Collision(other).hit(this)) {
dieDueToCollision(trans)
}
}
And we are green, and down to as little duplication as possible. Better commit: refactor to checkCollision.
What else have we? Missile, I guess.
fun interact(asteroid: Asteroid, trans: Transaction) {
if (checkCollision(asteroid)) {
if (missileIsFromShip) trans.addScore(asteroid.getScore())
terminateMissile(trans)
}
}
fun interact(saucer: Saucer, trans: Transaction) {
if (checkCollision(saucer)) {
if (missileIsFromShip) trans.addScore(saucer.getScore())
terminateMissile(trans)
}
}
fun interact(ship: Ship, trans: Transaction) {
if (checkCollision(ship)) terminateMissile(trans)
}
fun interact(missile: Missile, trans: Transaction) {
if (checkCollision(missile)) terminateMissile(trans)
}
I have an idea for this one. I don’t quite see how to do it with the refactoring tools. I’ll just do it:
private fun checkCollisionAndOptionallyScore(
other: Collider,
trans: Transaction,
action: (Transaction) -> Unit){
if ( checkCollision(other)) {
terminateMissile(trans)
action(trans)
}
}
New function, has an action. I use it like this:
fun interact(asteroid: Asteroid, trans: Transaction) {
checkCollisionAndOptionallyScore(asteroid, trans) {
if (missileIsFromShip) trans.addScore(asteroid.getScore())
}
}
Now that I think about it, I could probably do this with a let
or run
or one of those.
Let’s try it this way first. We are green, and I think we’re OK.
Plug it in the other cases:
fun interact(asteroid: Asteroid, trans: Transaction) {
checkCollisionAndOptionallyScore(asteroid, trans) {
if (missileIsFromShip) trans.addScore(asteroid.getScore())
}
}
fun interact(saucer: Saucer, trans: Transaction) {
checkCollisionAndOptionallyScore(saucer, trans) {
if (missileIsFromShip) trans.addScore(saucer.getScore())
}
}
fun interact(ship: Ship, trans: Transaction) {
checkCollisionAndOptionallyScore(ship, trans) {}
}
fun interact(missile: Missile, trans: Transaction) {
checkCollisionAndOptionallyScore(missile, trans) {}
}
Should be green. We are. I can commit, so I shall. Commit: refactoring Missile interaction.
However, I still think I can do the same thing with an also
or one of the other ones. Let’s try that.
No, wait. Belay that. It won’t work because we need to do the action only if the collision occurred.
I hate the name. What would be better? Maybe this:
private fun checkCollisionAndDoIfColliding(
other: Collider,
trans: Transaction, action:
(Transaction) -> Unit){
if ( checkCollision(other)) {
terminateMissile(trans)
action(trans)
}
}
I think we don’t really need to pass the transaction, because we have it at the calling level.
private fun checkCollisionAndDoIfColliding(
other: Collider,
trans: Transaction,
action: () -> Unit){
if ( checkCollision(other)) {
terminateMissile(trans)
action()
}
}
Compiles. Green. Commit: simplify and rename checkCollisionAndSoIfColliding (arrgh).
Enough for this afternoon. Let’s sum up.
Summary
We have renamed all the calls in Interaction to interact
. That name is overloaded in all the implementing classes, which allows them to behave differently in response to different interactions. Two classes use differing responses, and in two classes, all the responses are the same.
We’ve made the implementations similar in all the objects, although not identical. It seems likely that we could remove a bit of duplication somehow, but I’ll save that possibility for later when, with any luck, I can get a little help from my friends.
The method with the optional behavior is odd and it has an odd name. Maybe that’s as it should be. Maybe it is too clever to live: I’ll get some advice on that from my friends.
For now … the code is a bit simpler, and I think, more clear. And that’s a good thing.
See you next time!