https://kotlinlang.org logo
#getting-started
Title
# getting-started
n

Nat Strangerweather

07/27/2022, 12:16 PM
Hi, this piece of code works fine but it looks odd to me (all the exclamation marks!)... I suspect I am not making the best use of the Kotlin language. Could you please give me your opinion? If it can be done better, what would I need to work on?
Copy code
val skillRating = MutableLiveData(0)
    fun calculateSkillRating() {
        if (readWinCount.value != 0 && readLossCount.value != 0) {
            skillRating.value = averageWinningScore.value?.plus(
                (400 * (readWinCount.value?.minus(readLossCount.value!!)!!) /
                        (readWinCount.value?.plus(readLossCount.value!!)!!))
            )
        }
    }
j

Jurriaan Mous

07/27/2022, 12:49 PM
If variables are mutable the type system needs castings to make sure they are not null. Better to put some values into variables so the type system can be sure they do not change during use. Also the elvis
?:
operator can be useful to set some values to 0 if they are null
Copy code
val skillRating = MutableLiveData(0)
    fun calculateSkillRating() {
        val readWinCountValue = readWinCount.value ?: 0
        val readLossCountValue = readLossCount.value ?: 0
        val averageWinningScoreValue = averageWinningScore.value ?: 0        
        if (readWinCountValue != 0 && readLossCountValue != 0) {
            skillRating.value = averageWinningScoreValue + 400 * (readWinCountValue - readLossCountValue) /
                        (readWinCountValue + readLossCount)
            
        }
    }
n

Nat Strangerweather

07/27/2022, 12:52 PM
Thanks, this does look nicer! 😊
s

Stephan Schroeder

07/27/2022, 12:54 PM
the explamation marks are the "trust me, this isn't null operator". I think "the code is running fine" means the values aren't null, because otherwise you'd get NullPointerExceptions. My solution would be:
Copy code
fun calculateSkillRating() {
        if (readWinCount.value != 0 && readLossCount.value != 0) {
          val avarageWinningScoreValue = checkNotNull(averageWinningScore.value)
          val readWinCountValue = checkNotNull(readWinCount.value)
          val readLossCountValue = checkNotNull(readLossCount.value)
            skillRating.value = averageWinningScoreValue + (400 * (readWinCountValue -readLossCountValue) /
                        (readWinCountValue + readLossCountValue)
            )
        }
    }
So @Jurriaan Mous chose to select a default value of 0 in case the variable is
null
while my code throws an
IllegalStateException
, the original code threw a
NullPointerException
if one of the properties was
null
which I assume wasn't handled anywhere, so technically my code is closer to the original implementation. You have to decide if using 0 instead is ok in the context of the code.
👍 2
🙏 1
n

Nat Strangerweather

07/27/2022, 1:02 PM
I think I handled the possibility of a
NullPointerException
with this in the main code:
Copy code
val skillRating by statsViewModel.skillRating.observeAsState(initial = 0)
But yes, 0 is the value I would want to start with.
d

davec

07/27/2022, 3:13 PM
I'll add that in general you pretty much never want to use
!!
in Kotlin, that's basically telling the compiler you know better than it does, and you're introducing the possibility of a
NullPointerException
. That's Java thinking! Instead, you should check if something is null before using it using any one of a number of mechanisms that Kotlin provides (
if
,
when
, elvis operator,
let
, etc.)
The other thing to consider is whether the values you're using really need to be nullable in the first place. In Java, nulls are everywhere, but in a lot of cases in Kotlin you don't need nulls at all.
n

Nat Strangerweather

07/27/2022, 3:22 PM
Thanks for these interesting points. I have tons of
!!
in my code and often get nullpointers. I think I need to read up on nullables. 😅
👏 1
d

davec

07/27/2022, 3:25 PM
One other suggestion: perhaps a better functional approach would be to have the function be a pure function (https://en.wikipedia.org/wiki/Pure_function) meaning
calculateSkillRating
should have input parameters and return a value rather than having side effects:
Copy code
fun calculateSkillRating(readWinCountValue: Int, readLossCountValue: Int, averageWinningScore: Int): Double =
    averageWinningScore + (400 * (readWinCountValue-readLossCountValue) /
                        (readWinCountValue + readLossCountValue))
(The code may not be exactly correct, I'm not sure of the types, but you get this idea) Here the purpose of the function is clear, easy to unit test, and gets rid of all the noisy error cases. You might also have a special case for when the win/loss count are zero, but I'd argue that none of those input parameters should be nullable, and that the function doesn't mutate state
skillRating.value = ...
but instead just does a simple calculation and you do the assignment elsewhere. Disclaimer though: I don't have your full source code, and this is sort of just a sketch of what I'd recommend.
🙏 1
👍 1