https://kotlinlang.org logo
#codereview
Title
# codereview
t

therealbluepandabear

03/09/2021, 3:10 AM
Can this code be shortened?
Copy code
fun getValue(value: Value, param: MutableMap<Char, Int>): Pair<Char, Int> {
    return when (value) {
        Value.MIN -> Pair(param.keys.elementAt(param.values.indexOf(param.values.minOrNull() ?: 0)), param.values.minOrNull() ?: 0)
        Value.MAX -> Pair(param.keys.elementAt(param.values.indexOf(param.values.maxOrNull() ?: 0)), param.values.maxOrNull() ?: 0)
    }
}
Thanks, Tom
a

aleksey.tomin

03/09/2021, 4:09 AM
Copy code
fun getValue1(value: Value, param: MutableMap<Char, Int>) =
        when (value) {
            Value.MIN -> param.values.minOrNull() ?: 0
            Value.MAX -> param.values.maxOrNull() ?: 0
        }

    fun getValue2(value: int) = Pair(param.keys.elementAt(param.values.indexOf(value)), value)

    fun getValue(value: Value, param: MutableMap<Char, Int>) = getValue2(getValue1(value, param))
e

ephemient

03/09/2021, 4:22 AM
Copy code
fun getEntry(value: Value, param: Map<Char, Int>): Pair<Char, Int>? {
    val entry = when (value) {
        Value.MIN -> param.entries.minByOrNull { it.value }
        Value.MAX -> param.entries.maxByOrNull { it.value }
    } ?: param.entries.find { it.value == 0 }
    return entry?.toPair()
}
the
?:
fallback is of dubious utility. the only case when there is no min/max value is when the map is empty, in which case you won't find a 0 value either.
I question the function, parameter, and type names as well, and if this is going to be a common operation you probably want a different data structure than a plain Map
t

therealbluepandabear

03/09/2021, 4:27 AM
@ephemient what data structure would you use instead?
e

ephemient

03/09/2021, 4:28 AM
depends on what your operations are
t

therealbluepandabear

03/09/2021, 4:28 AM
@ephemient its just a simple application I am making... I am analysing some data; just for fun.
e

ephemient

03/09/2021, 4:30 AM
at the very least, this single function alone suggests indexing the input by what is currently the map's values. so perhaps a SortedMap<Int, Set<Char>>, perhaps with an auxiliary Set<Char> to maintain the former keys' uniqueness
if you need lookup in both directions, then build a bimap
t

therealbluepandabear

03/09/2021, 4:30 AM
@ephemient ok. thanks for this information.
e

ephemient

03/09/2021, 4:30 AM
at the very least, you can write this particular function without double linear traversal of all the entries
t

therealbluepandabear

03/09/2021, 4:31 AM
@ephemient ok thanks
e

elect

03/09/2021, 12:58 PM
Copy code
fun getValue(value: Value, param: MutableMap<Char, Int>): Pair<Char, Int> {
    val value = when (value) {
        Value.MIN -> param.values.minOrNull()
        Value.MAX -> param.values.maxOrNull()
    } ?: 0
    return param.keys[param.values.indexOf(value)] to value 
}
Copy code
operator fun MutableMap<Char, Int>.get(value: Value): Pair<Char, Int> {
    val value = when (value) {
        Value.MIN -> values.minOrNull()
        Value.MAX -> values.maxOrNull()
    } ?: 0
    return keys[values.indexOf(value)] to value 
}
e

ephemient

03/09/2021, 5:00 PM
indexOf() is still a second scan that you don't need. the key point is that
entries.minByOrNull { it.value }
entries.maxByOrNull { it.value }
gives you both the key and value at once
also MutableMap is unnecessary here
t

therealbluepandabear

03/10/2021, 12:59 AM
@ephemient yes but I was using a mutable map in the code in 'MAIN' what can I use instead? Cheers, Tom
e

ephemient

03/10/2021, 2:46 AM
@therealbluepandabear you can define it on
Map<>
, it will accept
MutableMap<>
as input
7 Views