Kotlin null check for multiple nullable var's

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

1 Like

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.

8 Likes

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

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?

1 Like

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.

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

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.

1 Like

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.

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

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

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) {
    
        }
    }
    
1 Like

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)
12 Likes

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

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.

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

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

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.

1 Like

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

4 Likes

Hey there,

i’ve run in this problem as well.
Before I wrote Stuff in Kotlin I used Scala and Clojure to get things done, both of them have a more elegant way to deal with multiple NonNull Values for Collection Operations and Lets, then Kotlin has out of the box. So I decided to use the power of ExtensionFunctions and wrote my own Library to make it more elegant and readable.

You can find the library on Github: GitHub - stupacki/MultiFunctions

The project is still in progress and I need to add a discription to use it but here a litte example to fix your problem.

import io.multifunctions.letNotNull

Quad(userApi.get(userId),
     ordersApi.get(userId),
     favoritesApi.get(userId),
     notesApi.get(userId))
    .letNotNull { user, orders, favorites, notes ->
    
         HttpResult.renderPage(user = user,
                               order = orders,
                               favorites = favorites,
                               notes = notes)

    }

In this example the lambda returns null in case one of the accessed apis return a null object, the let will return a null as well.

Currently the lib is able to handle 6 parallel objects/collections with the following functions:

  • let
  • letNotNull
  • map
  • mapNotNull
  • forEach
  • forEachNotNull
  • mapIndexed
  • mapIndexedNotNull
  • withIndex

I hope this will help you with this problem

Despite possible performance impacts, local vals is the best solution. If the variable is mutable and accessible to outer scopes then the only way to guarantee that it hasn’t changed is making a local copy. Even null checking using ?.let {} for single variables does this:

fun test() {
    name?.let { doSomething(it) }
}

compiles to the equivalent java code:

public final void test() {
    String var10000 = this.name;
    if(this.name != null) {
        String var1 = var10000;
        this.doSomething(var1);
    }
}

(Note: the kotlin compiler does optimise this when it can be sure that the variable won’t change. If name was a local var instead of a property, the compiler wouldn’t make an additional local variable

Swift has a similar problem with null checking multiple vars and solves this by making it easy to declare local values within if statements:

// Swift code 
if let name = name, let age = age {
    doSth(name, age)
}

A similar syntax in Kotlin could be the following:

if (val name = name, val age = age) {
    doSth(name, age)
}

Which would compile to the equivalent java code:

final String name = this.name;
if (name != null) {
    final Integer age = this.age;
    if (age != null) {
        doSomething(name, age);
    }
}

The usage of the comma instead of && is to differentiate it from normal if statements in that everything should evaluate to non-null, the || operator is not allowed and do not make sense:

if (val name = name || val age == age) { /* Defeats the point of null checking */ }

Like with the ?.let{} case, it would be possible for the compiler to optimise the creation of the local variables, only creating them if required. Also like the it in the ?.let{} block, any values declared in the scope of the of the if statement and it’s following block would be immutable.

The syntax could be generalised as follows:

if (val var1 = <expression1> ,val var2 = <expression2>, val varN = <expressionN>) {
    // Do something with var1, var2, varN
}

Where <expressionX> is any expression that returns a nullable type

Which compiles to the equivalent java code:

final Type1 var1 = <expression1>;
if (var1 != null) {
    final Type2 var2 = <expression2>;
    if (var2 != null) {
        final TypeN varN = <expressionN>;
        if (varN != null) {
            // Do something with var1, var2, varN
        }
    }
}

It may seem a bit much, but it is almost identical to how multiple consecutive safe null calls are compiled. e.g: foo?.bar?.baz

The benefit however is that is that you can use earlier values in later expressions. In the example below p1 and p2 have already been null checked so we can access their age property directly.

fun whoIsOldest(person1: Person?, person2: Person?) {
    if (val p1 = person1, val p2 = person2, val age1 = p1.age, age2 = p2.age) {
        when {
            age1 > age2 -> print("$p1 is oldest")
            age1 < age2 -> print("$p2 is oldest")
            else -> print("$p1 and $p1 are the same age")
        }
    } else {
        print("Your need two people to compare ages")
    }
}

And because the expressions can be anything that returns a nullable type it can also be used with safe casts:

if (val child = person as? Child, val car = child.favouriteToy as? Car) {
    car.race()
    print("$child is racing their toy $car")
}

A additional improvement that could be made which is also available in swift is allowing boolean expressions alongside the nullable expressions. It’s a fairly common use case to check if something is non-null and then perform an additional check to see if it is appropriate to use.

if (val childAge = person.child?.age, childAge >= 6 && childAge < 18) {
    print("$person has to drop their child at school each weekday")
}

Here the use of the comma instead of && reinforces the requirement that every expression needs to evaluate to non-null and true, no || operators are allowed.

8 Likes