Kotlin null check for multiple nullable var's


#1

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:

  1. use let

     fun test(){
     	name?.let { name ->
     		age?.let { age ->
     			doSth(name, age) 
     		}
     	}
     }
    
  2. use local variables

     fun test(){
     	val name = name
     	val age = age
     	if(name != null && age != null){
     		doSth(name, age)
     	}
    
     }
    
  3. 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?


#2

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)
    }

#3

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


#4

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.


#5

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


#6

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?


#7

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.


#8

@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.


#9

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.


#10

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.


#11

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).


#12

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)
}

#13

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

  1. 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) {
    
        }
    }
    
  2. 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) {
    
        }
    }
    
  3. 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) {
    
        }
    }
    

#14

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)

#15

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


#16

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.


#17

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


#18

In this case, I would use !! without discomfort :stuck_out_tongue:


#19

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.


#20

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