GitHub Repo

I think I’ve thought of a defect. Let’s find out.

When I looked at IDEA, it was open on OneShot, and I see an opportunity for improvement, and what I think is a defect.

class OneShot(private val delay: Double, private val action: (Transaction)->Unit) {
    var triggered = false
    var deferred: DeferredAction? = null
    fun execute(trans: Transaction) {
        if (!triggered) {
            triggered = true
            deferred = DeferredAction(delay, trans) {
                triggered = false
                action(it)
            }
        }
    }
    
    fun cancel(trans: Transaction) {
        if (deferred != null) trans.remove(deferred!!)
    }
}

As it stands, if the OneShot is triggered, and then cancelled, it can’t be triggered again. We need a test. Presently we have none. Let’s make some.

class OneShotTest {
    @Test
    fun `runs in specified time`() {
        val trans = Transaction()
        var count = 0
        val once = OneShot(2.0) { count += 1}
        assertThat(count).isEqualTo(0)
        once.execute(trans)
        val defer = trans.firstAdd()
        defer.update(0.1, trans)
        assertThat(count).isEqualTo(0)
        defer.update(2.2, trans)
        assertThat(count).isEqualTo(1)
    }
}

This one passes. Let’s test triggered. In this next one, let’s test in a mix. No, that’s an infinite hassle. Let’s test more directly.

    @Test
    fun `cancel removes deferred`() {
        val trans = Transaction()
        var count = 0
        val once = OneShot(2.0) { count += 1}
        assertThat(count).isEqualTo(0)
        once.execute(trans)
        val defer = trans.firstAdd()
        val remove = Transaction()
        once.cancel(remove)
        assertThat(remove.firstRemove()).isEqualTo(defer)
    }

These are all running, so far. Now let’s get tuff:

    @Test
    fun `cancel allows another shot`() {
        val trans = Transaction()
        var count = 0
        val once = OneShot(2.0) { count += 1}
        assertThat(count).isEqualTo(0)
        once.execute(trans)
        val defer = trans.firstAdd()
        val remove = Transaction()
        once.cancel(remove)
        assertThat(remove.firstRemove()).isEqualTo(defer)
        val newOne = Transaction()
        once.execute(newOne)
        val newDefer = newOne.firstAdd()
    }

If this works, I’ll get a. new add. I predict that it will not work.

Empty list doesn't contain element at index 0.

Now my fix:

    fun cancel(trans: Transaction) {
        if (deferred != null) {
            triggered = false
            trans.remove(deferred!!)
        }
    }

I expect the test to run. It does. Commit: fix undiscovered defect preventing oneShot from triggering after a cancel.

Now let’s improve this code. I think it’ll go better if we have a variable ready instead of triggered but since we now have to hold onto the deferred object, maybe it’s better just to use it:

class OneShot(private val delay: Double, private val action: (Transaction)->Unit) {
    var deferred: DeferredAction? = null
    fun execute(trans: Transaction) {
        if (deferred == null) {
            deferred = DeferredAction(delay, trans) {
                action(it)
            }
        }
    }

    fun cancel(trans: Transaction) {
        if (deferred != null) {
            trans.remove(deferred!!)
            deferred = null
        }
    }
}

This runs. Commit: remove triggered flag from OneShot, used deferred’s state.

We can be a bit more idiomatic … but i see a new defect: we’re not resetting deferred to null when it takes action. We’re missing a test.

    @Test
    fun `will run twice after action`() {
        val trans = Transaction()
        var count = 0
        val once = OneShot(2.0) { count += 1}
        assertThat(count).isEqualTo(0)
        once.execute(trans)
        val defer = trans.firstAdd()
        val removeTrans = Transaction()
        defer.update(2.1, removeTrans)
        assertThat(count).isEqualTo(1)
        assertThat(removeTrans.firstRemove()).isEqualTo(defer)
        val another = Transaction()
        once.execute(another)
        val newDefer = another.firstAdd()
    }

This fails on the final firstAdd because of the bug I’m after. Fix it:

class OneShot(private val delay: Double, private val action: (Transaction)->Unit) {
    var deferred: DeferredAction? = null
    fun execute(trans: Transaction) {
        deferred = deferred ?: DeferredAction(delay, trans) {
            deferred = null // <--- the fix
            action(it)
        }
    }

    fun cancel(trans: Transaction) {
        deferred?.let { trans.remove(it); deferred = null }
    }
}

Green. Commit: fix unexercised defect in OneShot that prohibited a second execute after first one had completed.

OK, that’s nice. I feel somewhat vindicated after this morning’s sluggish start. I’m also happy to have learned the trick for assigning only if the variable is null. I like to know and use idiomatic language features like that. The Kotlin people put a lot of thought into it, the least we can do is use it well.

That’ll do for this afternoon. Hit it again tomorrow!