CodeSOD: Null Thought

These days, it almost seems like most of the developers writing code for the Java Virtual Machine aren't doing it in Java. It's honestly an interesting thing for programming language development, as more and more folks put together languages with fundamentally different principles from Java that still work on the JVM.

Like Kotlin. Kotlin blends functional programming styles with object-oriented principles and syntactic features built around writing more compact, concise code than equivalent Java. And it's not even limited to Java- it can compile down into JavaScript or even target LLVM.

And since you can write bad code in any language, you can write bad code in Kotlin. Keith inherited a Kotlin-developed Android app.

In Kotlin, if you wanted to execute some code specifically if a previous step failed, you might use a try/catch exception handler. It's built into Kotlin. But maybe you want to do some sort of error handling in your pipeline of function calls. So maybe you want something which looks more like:

response.code .wasSuccess() .takeIf { false } ?.run { doErrorThing(it) } 

wasSuccess in this example returns a boolean. The takeIf checks to see if the return value was false- if it wasn't, the takeIf returns a null, and the run call doesn't execute (the question mark is our nullable operator).

Kotlin has a weird relationship with nulls, and unless you're explicit about where you expect nulls, it is going to complain at you. Which is why Keith got annoyed at this block:

/** * Handles error and returns NULL if an error was found, true if everything was good */ inline fun Int.wasSuccessOrNull() : Boolean? { return if (handleConnectionErrors(this)) null else true } /** * Return true if any connection issues were found, false if everything okay */ fun handleConnectionErrors(errorCode: Int) : Boolean { return when (errorCode) { Error.EXPIRED_TOKEN -> { requiresSignIn.value = true; true} Error.NO_CONNECTION -> { connectionIssue.value = true; true} Error.INACTIVE_ACCOUNT -> { inactiveAccountIssue.value = true; true} Error.BAD_GATEWAY -> { badGatewayIssue.value = true; true} Error.SERVICE_UNAVAILABLE -> { badGatewayIssue.value = true; true} else -> { if (badGatewayIssue.value == true) { badGatewayIssue.value = false } noErrors.value = true false } } }

wasSuccessOrNull returns true, if the status code is successful, otherwise it returns" null? Why a null? Just so that a nullable ?.run" call can be used? It's a weird choice. If we're just going to return non-true/false values from our boolean methods, there are other options we could use.

But honestly, handleConnectionErrors, which it calls, is even more worrisome. If an error did occur, this causes a side effect. Each error condition sets a variable outside of the scope of this function. Presumably these are class members, but who knows? It could just as easily be globals.

If the error code isn't an error, we explicitly clear the badGatewayIssue, but we don't clear any of the other errors. Presumably that does happen, somewhere, but again, who knows? This is the kind of code where trying to guess at what works and what doesn't is a bad idea.

[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.