Count sum of all list elements until condition is satisfied

Viewed 545

I faced the following problem: I have a list of objects. Let it be objects of the class Test:

data class Test(
    var status: String, // Can be EXPIRED, WAIT
    var amount: Float
)

The array is sorted, there are objects with the status EXPIRED in the beginning and after objects with the status WAIT located. I need to calculate the sum of all elements with the status EXPIRED (if they exist) and add to this sum amount of the first object with the type WAIT (if it exists). Now I have the following code:

 private fun getRestructuringAmountToPay(): Float {
        var i = 0
        var sum = 0F
        list?.forEachIndexed { iter, el ->
            if (el.status != RestructingItemStatus.WAIT) {
                i = iter
                sum += el.amount ?: 0F
            }
        }
        if (i + 1 < (list?.size ?: 0)) {
            sum += list?.get(i+1)?.amount ?: 0F
        }
        return sum
    }

But is there any way to improve this code and make it Kotlin-pretty? Thanks in advance for any help!

6 Answers

since your list is sorted and EXPIRED items are first you can use firstOrNull to find the first item with status == WAIT

while you iterate over EXPIRED items you can use a simple variable to sum the amount and when you found the first WAIT item just assign the sum to amount

 var sum: Float = 0f
 list.firstOrNull {
     sum += it.amount
     it.status == "WAIT"
 }?.apply {
     this.amount = sum
 }

I would go old school and use a for loop rather than the forEachIndexed method so that I can break out of the loop when I hit the first WAIT entry. I'd do something like this:

private fun getRestructuringAmountToPay(): Float {
    var sum = 0F
    for (el in list) {
        if (el.status == RestructingItemStatus.WAIT) {
            el.amount += sum
            break
        }
        else {
            sum += el.amount
        }
    }
    return sum
}

This is a simple and elegant way to do the bare minimum amount of work without any extra iterations over the list. If you're one of the "I have to cram everything into as few lines of code as possible" crowd, there are certainly more sophisticated and compact ways to go about this. I often struggle to understand the actual advantage of such solutions.

When you say you want to find all of the elements in the list with the status "EXPIRED", that makes me think of filter(). When you say you want to sum them, that makes me think of sumBy(). And when you say you want to add that number to the first element in the list with the status "WAIT", that makes me think of first().

We can't actually use the normal sumBy() function because Test.amount is of type Float, so the closest we can do is use sumByDouble() and convert amount to a Double.

val expiredSum = list.filter { it.status == "EXPIRED" }.sumByDouble { it.amount.toDouble() }
val result = list.first { it.status == "WAIT" }.amount + expiredSum

If you don't want to throw an exception if there are no elements with the status "WAIT", use firstOrNull() instead:

val expiredSum = list.filter { it.status == "EXPIRED" }.sumByDouble { it.amount.toDouble() }
val initialValue = list.firstOrNull { it.status == "WAIT" }?.amount ?: 0F
val result = initialValue + expiredSum

Actually, your code is not doing what you want

calculate the sum of all elements with the status EXPIRED (if they exist) and add to this sum amount of the first object with the type WAIT (if it exists)

It calculates the sum of all elements with the status EXPIRED (if they exist) and add to this sum amount of the first object with the type WAIT located after the last object with EXPIRED status (if it exists) OR amount of the object with index one (if it exist) if there were no elements with status EXPIRED:

println(getRestructuringAmountToPay(listOf(Test("EXPIRED", 1f), Test("WAIT", 1f), Test("EXPIRED", 1f)))) //will print 2.0, while following original description it should be 3.0
println(getRestructuringAmountToPay(listOf(Test("WAIT", 1f), Test("WAIT", 100f)))) //Will print 100.0, while following original description it should be 1.0

To get originally desired behavior in Kotlin-way you need to do the following:

if (list == null) return 0f //get rid of nullability
val (expired, waiting) = list.partition { it.status != "WAIT" } //split original list into respectful partitions
val sum = expired.map { it.amount }.sum() + (waiting.firstOrNull()?.amount ?: 0f) //do the calculations

I'am not sure I understand your solution but respectively to your goal this:

    var sum = list.filter { it.status == "EXPIRED" }.sumByDouble { it.amount.toDouble() }
    list.firstOrNull{ it.status == "WAIT" }?.let { sum+=it.amount}

    println(sum)

Pretty is subjective - the code below is short but admittedly does not take advantage of the "sorted" nature of the collection

fun sum(data: List<Test>): Double {
    val expiredSum = data.filter { it.status == "EXPIRED" }.sumByDouble { it.amount }
    val waitSum = data.find { it.status == "WAIT" }?.amount ?: 0.0
    return expiredSum + waitSum
}

Related