Arithmetic Parser in Kotlin

Question

I don't really have a problem. But, it's my first time using Kotlin for any project, so I want to know if there is any problem in my code or my code can be made cleaner.

This is an arithmetic parser made in Kotlin. It can evaluate a expression like "(6+4)/(2+3)" to 2.0 This handles operations like

1. Power(^)
2. Divison(/)
3. Multiplication(*)
4. Subtraction(-)
5. Addition(+)

It also handles brackets.

My Kotlin Code is :-

import kotlin.math.pow

fun basic(rightNum:String?, leftNum:String?, op:String?):Double? {
    return when (op) {
        "+" -> {
            (rightNum?.toDouble()!! + leftNum?.toDouble()!!)
        }
        "-" -> {
            (rightNum?.toDouble()!! - leftNum?.toDouble()!!)
        }
        "*" -> {
            (rightNum?.toDouble()!! * leftNum?.toDouble()!!)
        }
        "^" -> {
            ((rightNum?.toDouble()!!).pow(leftNum?.toDouble()!!))
        }
        else -> {
            (rightNum?.toDouble()!! / leftNum?.toDouble()!!)
        }
    }
}


fun elemInside(mainString:String?, listCheck:List<String>):Boolean {
    for (ops in listCheck) {
        if (mainString?.contains(ops)!!){
            return true
        }
    }
    return false
}

fun getOpIndex(query: String?, operations:List<String>):Array<Int> {
    var allIndex:Array<Int> = arrayOf()
    var dupQuery = query
    while (elemInside(dupQuery, operations)) {
        for (op in operations) {
            if (dupQuery?.contains(op)!!) {
                allIndex = allIndex.plusElement(dupQuery.indexOf(op))
                dupQuery = dupQuery.substring(0, dupQuery.indexOf(op)) + '1' + dupQuery.substring(dupQuery.indexOf(op) + 1)
            }
        }
    }

    allIndex.sort()
    return allIndex
}


fun parseSimple(query:String?):Double? {
    val operations = listOf("^", "/", "*", "-", "+")
    var allIndex: Array<Int> = arrayOf()

    var calcQuery = query
    while (elemInside(calcQuery, operations) && (allIndex.size > 1 || if (allIndex.isEmpty()) true else allIndex[0] != 0)) {
        for (op in operations) {
            calcQuery = calcQuery?.replace("-+", "-")
            calcQuery = calcQuery?.replace("--", "+")
            calcQuery = calcQuery?.replace("+-", "-")
            allIndex = getOpIndex(calcQuery, operations)
            if (calcQuery?.contains(op)!!) {
                val indexOp = calcQuery.indexOf(op)
                val indexIndexOp = allIndex.indexOf(indexOp)
                val rightIndex =
                    if (indexIndexOp == allIndex.lastIndex) calcQuery.lastIndex else allIndex[indexIndexOp + 1]
                val leftIndex = if (indexIndexOp == 0) 0 else allIndex[indexIndexOp - 1]
                val rightNum =
                    calcQuery.slice(if (rightIndex == calcQuery.lastIndex) indexOp + 1..rightIndex else indexOp + 1 until rightIndex)
                val leftNum = calcQuery.slice(if (leftIndex == 0) leftIndex until indexOp else leftIndex + 1  until indexOp)
                val result = basic(leftNum, rightNum, op)
                calcQuery = (if (leftIndex != 0) calcQuery.substring(
                    0,
                    leftIndex + 1
                ) else "") + result.toString() + (if(rightIndex != calcQuery.lastIndex) calcQuery.substring(
                    rightIndex..calcQuery.lastIndex
                ) else "")
            }
        }
    }
    return calcQuery?.toDouble()
}

fun getAllIndex(query: String?, char: Char, replacement:String="%"):List<Int> {
    var myQuery = query
    var indexes:List<Int> = listOf()
    while (char in myQuery!!) {
        val indexFinded = myQuery.indexOf(char)
        indexes = indexes.plus(indexFinded)
        myQuery = myQuery.substring(0 until indexFinded) + replacement + myQuery.substring(indexFinded+1..myQuery.lastIndex)
    }
    return indexes
}

fun getBrackets(query: String?): List<Int> {
    val allEndIndex = getAllIndex(query, ')')
    val allStartIndex = getAllIndex(query, '(')
    val firstIndex = allStartIndex[0]
    for (endIndex in allEndIndex) {
        val inBrac = query?.substring(firstIndex+1 until endIndex)
        val inBracStart = getAllIndex(inBrac, '(')
        val inBracEnd = getAllIndex(inBrac, ')')
        if (inBracStart.size == inBracEnd.size){
            return listOf(firstIndex, endIndex)
        }
    }
    return listOf(-1, -1)
}

fun evaluate(query:String?):Double? {
    var calcQuery = query
    var index = 0;
    // Check if brackets are present
    while (calcQuery?.contains('(')!! && index < 200){
        val startBrackets = getBrackets(calcQuery)[0]
        val endBrackets = getBrackets(calcQuery)[1]
        val inBrackets = calcQuery.slice(startBrackets+1 until endBrackets)
        if ('(' in inBrackets && ')' in inBrackets){
            val inBracValue = evaluate(inBrackets)
            calcQuery = calcQuery.substring(0, startBrackets) + inBracValue.toString() + (if(endBrackets == calcQuery.lastIndex) "" else calcQuery.substring(endBrackets+1..calcQuery.lastIndex))
        }
        else {
            val inBracValue = parseSimple(inBrackets)
            calcQuery = calcQuery.substring(0, startBrackets) + inBracValue.toString() + (if(endBrackets == calcQuery.lastIndex) "" else calcQuery.substring(endBrackets+1..calcQuery.lastIndex))
        }
        index++
    }

    return parseSimple(calcQuery)
}


fun main() {
    print("Enter the equation: ")
    val equation = readLine()
    println(evaluate(equation))
}

Please tell me how I can improve the code.

The github link is here: https://github.com/ProgrammerPro94/ArithematicParserKotlin

Answer 1

You are using a lot of nullable types, combined with non-null assertions (!!). This defeats the purpose of using the nullable types in the first place. You should as early as possible check whether or not a value is null, and then pass it on as not-null.

For example, just looking at some of your function headers:

fun evaluate(query:String?):Double?

fun parseSimple(query:String?):Double?

fun basic(rightNum:String?, leftNum:String?, op:String?):Double?

Do these methods even make sense if any of those parameters is null? No! So don't declare them as nullable.

---

If I write 2^5 and you have variables called leftNum and rightNum, I would expect 2 to be left and 5 to be right. But your code is rightNum.toDouble().pow(leftNum.toDouble()) and it computes correctly. That's because you're putting 2 as rightNum and 5 as leftNum for some reason.

---

You can make better use of Kotlin's amazing API, for example in this method:

fun elemInside(mainString:String?, listCheck:List<String>):Boolean {
    for (ops in listCheck) {
        if (mainString?.contains(ops)!!){
            return true
        }
    }
    return false
}

This could be:

fun elemInside(mainString:String, listCheck: List<String>): Boolean {
   return listCheck.any { mainString.contains(it) }
}

Which can even be written as:

fun elemInside(mainString:String, listCheck: List<String>): Boolean
   = listCheck.any { mainString.contains(it) }

---

I would strongly recommend using the Shunting-yard Algorithm to parse the expression. It would enable you to implement new features with new operators and even functions such as sin, cos, sqrt, and so on...

Or even negative numbers, which you don't support right now. -2*3 breaks. It has to be written as (0-2)*3 in order to work. Using Shunting-yard Algorithm also allows you to deal with whitespace much easier.

Order of operations is also a bit of an issue with your current approach, 2*3+4*5 returns 50.0 while I would expect it to return 6+20 = 26. Shunting-yard would help with this too.