How to simplify when expression in kotlin

Question

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)
    }
}

The implementation of my own Stack Interface: `interface Stack { val storage: MutableList 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) } }` — halotukozak, Aug 22 '22 at 16:14
@k314159 i've already added more code to question. addToLast is a function, which adds incoming char (digit or letter) to the last of the output if both are the same type (digit or letter) — halotukozak, Aug 22 '22 at 16:22

score 2 · Accepted Answer · answered Aug 22 '22 at 16:28

2

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.

answered Aug 22 '22 at 16:28

user3738870

1,415
2
12
24

1

Ok, I see. Probablu i won't use it, but you helped me understood the function as parameter (conditionOnLast). I thoght that the condition should be a parameter but I have no idea how to do it. Thanks! – halotukozak Aug 22 '22 at 16:32
1

I've change the addToLast function in InputStack for addToLastConditionally. The main case is simplified, the number of functions stayed – halotukozak Aug 22 '22 at 16:34
1

Great, you're welcome! – user3738870 Aug 22 '22 at 16:35
2

Yeah this is the neatest way I think - I reckon it would be a little nicer as a `pushChar()` function though, with an *optional* predicate, so every case in the `when` can call it. You could either make the predicate nullable with a *null* default value (and skip using it if it's *null*, just do `output.push(char)`) or keep it non-nullable and give the predicate a default value of `{ true }`. I feel like the nullable version would be the way to go with the logic in here, but then all your branches could be written like `pushChar(char, output)`, `pushChar(char, output) { it.isNumeric() }`, etc – cactustictacs Aug 22 '22 at 19:37

score 0 · Answer 2 · answered Aug 22 '22 at 16:41

The new when expression:

smoothInput.forEach { char ->
            when {
                char.isValidOperator() -> output.push(char)
                char.isDigit() -> output.addToLastConditionally(char) { it.isNumeric() }
                char.isLetter() -> output.addToLastConditionally(char) { it.isValidVariableName() }
                else -> throw InvalidIdentifierException()
            }
        }

I've change the addToLast function in InputStack for addToLastConditionally

fun addToLastConditionally(char: Char, condition: (String) -> Boolean) {
        if (isNotEmpty() && condition(last()!!)) storage[storage.size - 1] += char.toString()
        else push(char)
    }

How to simplify when expression in kotlin

2 Answers2