kotlinsimplifykotlin-when

How to simplify when expression in kotlin


I'd like to simplify this expression, especially "isDigit" and "isLetter" case. How to do it?

smoothInput.forEach { char ->
            when {
                char.isValidOperator() -> {
                    output.push(char)
                }

                char.isDigit() -> {
                    if (output.isNotEmpty() && output.last()!!.isNumeric()) output.addToLast(char)
                    else output.push(char)
                }

                char.isLetter() -> {
                    if (output.isNotEmpty() && output.last()!!.isValidVariableName()) output.addToLast(char)
                    else output.push(char)
                }

                else -> {
                    throw InvalidIdentifierException()
                }
            }
        }

I think, that it isn't important, but it's much better to add code here than in comment output is InputStack Type:

class InputStack : Stack<String> {

    override val storage = mutableListOf<String>()
    fun push(e: Char) = push(e.toString())
    fun push(e: Operator) = push(e.toString())

    fun addToLast(e: Char) {
        storage[storage.size - 1] += e.toString()
    }
}

Stack Interface:

interface Stack<T> {
    val storage: MutableList<T>
    fun asString(): String = buildString {
        appendLine("----top----")
        storage.asReversed().forEach {
            appendLine(it)
        }
        appendLine("-----------")
    }

    fun push(element: T) = storage.add(element)
    fun pop(): T {
        if (storage.size == 0) throw EmptyStackException()
        return storage.removeAt(storage.size - 1)
    }

    fun isEmpty(): Boolean = storage.isEmpty()
    fun isNotEmpty(): Boolean = !isEmpty()
    fun last(): T? = storage.lastOrNull()
    fun forEach(action: (T) -> Unit) {
        for (element in storage) action(element)
    }
}

Solution

  • You can extract some common parts in the following way:

    fun addCharToOutputConditionally(char: Char, output: InputStack, conditionOnLast: (String) -> Boolean) {
        if (output.isNotEmpty() && conditionOnLast(output.last()!!)) output.addToLast(char)
        else output.push(char)
    }
    
    smoothInput.forEach { char ->
        when {
            char.isValidOperator() -> {
                output.push(char)
            }
    
            char.isDigit() -> {
                addCharToOutputConditionally(char, output) {
                    it.isNumeric()
                }
            }
    
            char.isLetter() -> {
                addCharToOutputConditionally(char, output) {
                    it.isValidVariableName()
                }
            }
    
            else -> {
                throw InvalidIdentifierException()
            }
        }
    }
    

    However, in cases like this, I don't think it's usually worth spending the time to refactor it this way, considering that there's little to gain by doing so: the resulting code is even longer and arguably harder to read than the original one.