I avoid using the not-null !!
operator because it reduces confidence that the code won't crash at runtime. And I also avoid list operations that can cause a crash if the list is empty, such as maxBy
(as opposed to maxByOrNull
). But sometimes avoiding both of these at the same time feels impossible. Here's an example:
data class Person(val name: String, val age: Int, val height: Int)
val people: List<Person> = // Initialize a list of people
println("Tallest person for each age:")
people.groupBy{it.age}.forEach{ age, peopleWithThisAge ->
val tallestPersonForThisAge = peopleWithThisAge.maxByOrNull { it.height }!!
println("$age: ${tallestPersonForThisAge.name}")
}
I know that peopleWithThisAge
will never be empty because groupBy
only creates lists with at least one value. But the complier doesn't know that. Is there an elegant way to handle this situation?
Even when I, as a human reading the code, know that the code won't crash, I want to avoid the mental burden of someone else having to read a bunch of code to understand that. If I wanted that mental burden, I would still be programming in Java.
CodePudding user response:
This is what some programming languages solve by using dependent types, that is, types coupled with values (like, a list type marked with the size).
Kotlin can't do that yet, and as long as the types of an empty and non-empty list are the same, the compiler can't really help with empty-checks unless it has some other tricks. The closest thing in the Kotlin language features to what could help here is contracts, but those don't have anything for empty/non-empty states of collections as of now.
Some community projects attempt to provide non-empty collection types for Kotlin:
quickbirdstudios/NonEmptyCollections
(see the blog post);gildor/knel
arrow-kt
– part ofarrow-core-data
Here's a feature request for non-empty collections in Kotlin: KT-21217
With the standard library, you can still somewhat reduce the burden on others reading your code by using asserting functions – check
, require
, checkNotNull
, requireNotNull
– and providing meaningful messages, like this:
people.groupBy { it.age }.forEach { age, peopleWithThisAge ->
val tallestPersonForThisAge = checkNotNull(
peopleWithThisAge.maxByOrNull { it.height }
) { "groupBy creates non-empty groups" }
println("$age: ${tallestPersonForThisAge.name}")
}
The functions check
and require
are marked with the according contracts: if they return without throwing an exception, the condition is satisfied. So the compiler can infer some conditions based on the calls to them, too.
CodePudding user response:
I think these are the choices you have in the standard library. Since it doesn't have non-empty Collection types, you unfortunately can't get compile-time safety, if that's what you mean by elegant.
Go ahead and use
maxBy()
. By using it you are asserting that you know it should be safe to use. I consider this is the clean way to do it when working with a collection in the same scope that you created it. Cleaner thanmaxByOrNull()!!
since the error message will be more appropriate if you were wrong.Maybe not super concise, but you can use
maxByOrNull()
with an Elvis operator and default. This might be elegant in situations where it's appropriate to have a default..maxByOrNull { it.height } ?: 0.0
Use
maxByOrNull()
with an Elvis and anerror
call. This is basically like usingmaxBy()!!
but with better messaging. `maxByOrNull { it.height } ?: error("collection was empty")
An improvement on no. 1 that builds on no. 3 might be to create a chain function for asserting the collection isn't empty. Something like:
fun <T> Iterable<T>.assertNotEmpty() =
(this as? Collection<T> ?: toList()).also {
require(it.isNotEmpty()) { "Not empty assertion failed." }
}
Then you could call this before calling maxBy
so later when you're reviewing code, the maxBy
call won't be as likely to raise eyebrows.