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

• programmer pro: Yes, I have nearly tested for around 100 expressions using lists of strings
• Mast: Do you have those tests automated? Or a list of them?
• programmer pro: I have made a list of around 100 strings with some hard expressions and some complications and then passes each of them as a value of a parser and then compare the results of the tests of the parser and a calculator result for that expressions. They were same.
• Simon Forsberg: @programmerpro I think you should learn about JUnit junit.org/junit5/docs/current/user-guide (It can be used from Kotlin too)
• Mast: Please do not update the code in your question after receiving answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .

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.

• programmer pro: Ok I will improve my code for negative numbers and also try to implement shunting yard algorithm.