Is there a way to clearly see whether an expression is being used as the "result" of a when?

Support
1

Hi,

First post, so apologies in advance. I am relatively new to Kotlin. It’s my favourite language.

I recently found a subtle bug in my code that evaded a couple of code reviewer keen eyes, so I thought it was worth posting a simplified version here as feedback/discussion on this part of the language and/or IDE.

class House {

    enum class State {
        DOOR_CLOSED, DOOR_OPEN
    }

    var state = State.DOOR_CLOSED

    fun processAction(action:String) {
        val newState = when (state) {
            State.DOOR_CLOSED -> {
                if (action == "openDoor") State.DOOR_OPEN
                State.DOOR_CLOSED
            }
            else -> State.DOOR_CLOSED // self-closing door :)
        }
        state = newState
    }

}

fun main(args: Array<String>) {
    var h = House() 
    h.processAction("openDoor")
    println("${h.state}, but could easily expect DOOR_OPEN")
}

It’s clear to me now that, of course, the line with State.DOOR_OPEN will not set the value of the when expression, because it’s not the last expression in that block. The if statement is missing an else clause.

However, since I’ve become accustomed to the rich and helpful advice of kotlin/IntelliJ, I half expected it would somehow make it obvious if it wasn’t working as I intended. Clearly this is a case of user error, but it would have been very helpful if there was some way to be able to verify whether a particular expression is considered a “return value” for a block like this.

I notice, in Android Studio at least, return expressions inside let blocks are annotated with a ^let tag, and something like this would have helped me a lot in this case.

Thoughts?

2

This would be something the IDE could/should warn about (you may wish to file a feature request). As State.DOOR_OPEN is a compile time constant it’s conditional referencing can be recognised as a problem reasonably easy. It has almost no side-effect (it can trigger class loading, the class initializer can have arbitrary side-effects - and yes that is very poor coding practice).