Kotlin null check for multiple nullable var's

panel · September 1, 2016, 5:35am

So let’s discuss this scenario

class Person(var name:String? = null, var age:Int? = null){
    fun test(){
        if(name != null && age != null)
            doSth(name, age) //smart cast imposible
    }

    fun doSth (someValue:String, someValue2:Int){

    }
}

When comes to nullity check for single nullable variable it’s just a meter of using let (simple & elegant solution)

name?.let{ doSth(it) }

But to handle the same scenario for multiple variables, requires a lot of boilerplate code. As I understand we have 3 options:

use let

 fun test(){
 	name?.let { name ->
 		age?.let { age ->
 			doSth(name, age) 
 		}
 	}
 }

use local variables

 fun test(){
 	val name = name
 	val age = age
 	if(name != null && age != null){
 		doSth(name, age)
 	}

 }

changing variables to be immutable

 data class Person(val name:String? = null, val age:Int? = null){
 	fun test(){
 		if(name != null && age != null){
 			doSth(name, age)
 		}
 	}
 }

The problem I have with all 3 above solutions is that they all are not as elegant and simple as check for single variable. I strongly feel they require to much boiler plate code (3rd options seems fine, but it’s not always possible to change variables to make them val).

I would like to propose alternative solution and I believe that it’s worth looking into. My proposal is to simply allow to use classic null check like this

if(name != null && age != null){
    doSth(name, age)
}

but generate local variables by the compiler underneath (2nd example). This way we make null check as simple as for val’s, make sure that variable will not be modified after nullity check and make code more concise.

What do you think?

medium · September 1, 2016, 8:06pm

What do you think of using a container like Pair and an extension function on Pair?

    fun main(args: Array<String>) {    
        val name: String? = "john"
        val age: Int? = 99
        Pair(name, age).let {
            println("name: $name, age: $age")
        }
    }

    fun Pair<String?, Int?>.let(action: (pair: Pair<String?, Int?>) -> Unit) {
        if (this.first != null && this.second != null)
            action(this)
    }

panel · September 2, 2016, 6:06am

Not good enough. Imagine this example with 3 or 4 variables.

Beholder · September 2, 2016, 7:15am

Always generating new local variables is not good for performance (and for semantic too).

Have another solution variant, but may say it’s also not very elegant:

inline fun <A, B, R> ifNotNull(a: A?, b: B?, code: (A, B) -> R) {
    if (a != null && b != null) {
        code(a, b)
    }
}
...
    fun test() {
        ifNotNull(name, age) { name, age ->
            doSth(name, age)
        }
    }

You may write overloaded functions ifNotNull for 3, 4, or more arguments.

panel · September 2, 2016, 10:41am

Thanks for the solution, I believe it’s the best I got so far in terms of semantics.

I totally do not agree that may proposed solution is:
a. bad for semantics, becouse compiller will generate this code for java bynary so semantics will not change
b. hurts performance, becouse the simplest way to make it work is to actually create local variables and this mean that they will be created one way or another, so it would be better if compiller create them automaticly underneatch instead of programmer createing them manually (better for code clarity, semantics)

I would love to hear from kotlin team about the idea

yole · September 2, 2016, 9:50pm

Could you please specify the exact rule that you suggest the Kotlin team to implement? When exactly should the compiler copy the data to those temporary local variables? What exactly should happen when the code later in the function modifies the value of one of those temporary variables? If another thread modifies the property that was copied into this temporary variable, how could the current thread get access to the new value?

mplatvoet · September 3, 2016, 8:45am

I know the following isn’t really an answer to your question but I do think it’s the proper solution.

I would rather avoid using nullable types altogether. Instead I prefer to rely on default parameters with sensible defaults. So just as an example the joinTo method, it takes loads of parameters but almost all have default value. This just avoids the null checks altogether.

With this approach through my codebases I find the need for such null checks have drastically reduced. Therefor I can live without a special language construct for the times I do run into these kind of issues.

panel · September 3, 2016, 9:21am

@mplatvoet you are right and I’ll definitely try to follow this way where possible, but you app does not working alone and communicating with external data sources sometimes require nulls.

@yole
1. When exactly should the compiler copy the data to those temporary local variables?
I would say the same logic should apply as for error “Smart cast to ‘String’ is impossible, because ‘name’ is a mutable property that could have been changed by this time”.
The goal is to allow programmer read value safely (without creating boilerplate code), but display error if programmer try to write value (from my experience reading is used much more often when writing and also there may be a case where you are reading few values but writing only one, so this one could be easily checked using let )

2. What exactly should happen when the code later in the function modifies the value of one of those temporary variables? If another thread modifies the property that was copied into this temporary variable, how could the current thread get access to the new value?

This indeed would be problematic, so I am generally assuming that this solution will create val temporary variables, so latter modification will not be a problem.

yole · September 3, 2016, 4:09pm

So do I understand you correctly that you propose to forbid modifying any var property of any object after it has been accessed for reading in the same function? Meaning that code as simple as if (foo.name != null) foo.name = "abc" will no longer compile?

This is of course absolutely unacceptable.

panel · September 5, 2016, 5:27pm

It would be absolutely unacceptable indeed. I had something else in mind, but I have analized the case thoroughly and it will not be possible to implement after all, sorry.

pdvrieze · September 7, 2016, 12:14pm

Unfortunately read only is not enough. As the value may be read multiple times and be used to write/create some other state you still have a race condition if the value changes in between. Unfortunately the only correct way is to explicitly define a local variable that can have exactly the needed semantics.

Kotlin is special in that it does flag possible race conditions in nullability, but that doesn’t mean that those issues are not just as valid for other values (flagging those would be very noisy though) that are not final. Of course, it also has issues with modifying vars.

The fundamental aspect though is that if you want to make correct decisions in a multithreaded context you will have to either use locking througout (I’m not sure if Kotlin will allow it, but it would be interesting if it would), or make a snapshot of the values (only locking the snapshotting).

maaartinus · September 11, 2016, 9:17pm

I may be pretty wrong, but let me try…

What about this: Allow the smart cast iff

it’d be allowed for a local variable
the variable isn’t volatile
the compiler can easily (i.e., in a simple and documented way) prove it doesn’t change before use

So

if (name != null && age != null) {
    doSth(name, age)
}

should compile and never throw NPE. This can be implemented via local variables without sacrificing any semantics as there’s no visibility guarantee in the JMM (lacking volatile and friends, the JIT is free to make a local copy).

This should work

even if doSth is a method modifying this.name as this happens after the call
even if doSth does some synchronization as it too happens after the call

Funnily, this means that then

if (name != null && age != null) {
    doSth(name, age)
    doSth(name, age)
}

would not compile (as during the first call, this.name may have changed or its change by other thread may have become visible according to the JMM). So only the most basic cases are covered, the question is if the remaining cases are rare enough for this to be worth doing.

Another idea would be introducing some shortcut variable declaration like

if (@localVal name != null && @localVal age != null) {
    doSth(name, age)
    doSth(name, age)
}

mfulton26 · September 26, 2016, 3:02pm

I see a few other options not yet discussed in case it helps:

Use an intermediary function to copy the property values thereby making smart cast possible:

class Person(var name: String? = null, var age: Int? = null) {
    fun test() = test(name, age)
    private fun test(name: String?, age: Int?) {
        if (name != null && age != null)
            doSth(name, age) //smart cast possible
    }

    fun doSth(someValue: String, someValue2: Int) {

    }
}

Use an anonymous function (same idea as #1):

class Person(var name: String? = null, var age: Int? = null) {
    fun test() = (fun(name: String?, age: Int?) {
        if (name != null && age != null)
            doSth(name, age) //smart cast possible
    })(name, age)

    fun doSth(someValue: String, someValue2: Int) {

    }
}

Use default arguments to copy the property values:

class Person(var name: String? = null, var age: Int? = null) {
    fun test(name: String? = this.name, age: Int? = this.age) {
        if (name != null && age != null)
            doSth(name, age) //smart cast possible
    }

    fun doSth(someValue: String, someValue2: Int) {

    }
}

electrolobzik · April 26, 2017, 8:01pm

I suppose new keyword let could be added to the language similar to Swift:

if let email = emailField?.text, let password = passwordField?.text {
}

It makes everything consistent:

it doesn’t break existing kotlin code or change it’s behaviour in any way
it gives easy and elegant way to make local not mutable variables to use inside the block
it also keep ability to access source property in any way (read/write)

ahmadalsharif.dev · May 23, 2017, 9:09am

This feels like introducing local immutable variables in Kotlin which theoretically impacts performance.

Wavesonics · June 11, 2017, 7:12am

If local immutable variables IS the solution, it would be nice to have some language level syntactical sugar to standardize and enforce it as a best practice.

Wavesonics · June 12, 2017, 11:37pm

would it be possible to do a version of the ifNotNull function with varargs? Thus we don’t need an infinite number of overloads?

kobe2000 · June 13, 2017, 1:15am

In this case, I would use !! without discomfort

yole · June 13, 2017, 8:03am

Varargs don’t work with type parameters, so such an API would have to use Any for all the types, which wouldn’t be nice to use.

Wavesonics · June 20, 2017, 7:37pm

The more I use Kotlin the more I wish for something similar to Swift’s guard/let convention.

Topic		Replies	Views
Proper usage of nullable vars after check for nullness	16	1723	January 19, 2015
Extension to ?.let syntax Language Design	7	2263	February 20, 2020
Local var nullibility	7	1785	August 11, 2017
Better null reference check?	5	1004	December 20, 2012
Are smart casts and null checking on global var thread safe?	3	6167	May 6, 2016

Kotlin null check for multiple nullable var's

Related topics