327

And the Kotlin newbie asks, "why won't the following code compile?":

var left: Node? = null
    
fun show() {
    if (left != null) {
        queue.add(left) // ERROR HERE
    }
}

Smart cast to 'Node' is impossible, because 'left' is a mutable property that could have been changed by this time

I get that left is mutable variable, but I'm explicitly checking left != null and left is of type Node so why can't it be smart-casted to that type?

How can I fix this elegantly?

iknow
  • 4,796
  • 9
  • 19
  • 37
FRR
  • 10,772
  • 13
  • 43
  • 80
  • 5
    Somewhere in between a different thread could have changed the value to null again. I'm pretty sure answers on the other questions mention that as well. – nhaarman Jun 16 '17 at 18:14
  • 4
    You could use a [safe call](https://kotlinlang.org/docs/reference/null-safety.html#safe-calls) to add – Whymarrh Jun 16 '17 at 18:16
  • thanks @nhaarman that makes sense, Whymarrh how can do that? I thought safe calls were only for objects not methods – FRR Jun 16 '17 at 18:19
  • 7
    Something like: `n.left?.let { queue.add(it) }` I think? – Jorn Vernee Jun 16 '17 at 18:21

10 Answers10

420

Between execution of left != null and queue.add(left) another thread could have changed the value of left to null.

To work around this you have several options. Here are some:

  1. Use a local variable with smart cast:

     val node = left
     if (node != null) {
         queue.add(node)
     }
    
  2. Use a safe call such as one of the following:

     left?.let { node -> queue.add(node) }
     left?.let { queue.add(it) }
     left?.let(queue::add)
    
  3. Use the Elvis operator with return to return early from the enclosing function:

     queue.add(left ?: return)
    

    Note that break and continue can be used similarly for checks within loops.

mfulton26
  • 24,821
  • 4
  • 55
  • 71
  • 10
    4. Think of a more functional solution to your problem that doesn't require mutable variables. – Good Night Nerd Pride Nov 11 '17 at 10:48
  • What is this n in n.left? – sak Jul 20 '18 at 11:47
  • 1
    @sak It was an instance of a `Node` class defined in the original version of the question which had a more complicated code snippet with `n.left` instead of simply `left`. I've updated the answer accordingly. Thanks. – mfulton26 Jul 20 '18 at 20:15
  • @mfulton26 Thanks for making it clear, but what if I have this error for 3 variables? e.g: "MyAsyncTask().execute(a1,a2,a3)" and here a1, a2, a3 all giving smart cast error. – sak Jul 21 '18 at 04:40
  • 1
    @sak The same concepts apply. You can create a new `val` for each `var`, nest several `?.let` statements, or use several `?: return` statements depending on your function. e.g. `MyAsyncTask().execute(a1 ?: return, a2 ?: return, a3 ?: return)`. You can also try one of the solutions for a ["multiple variable let"](https://stackoverflow.com/q/35513636/3255152). – mfulton26 Jul 21 '18 at 13:47
  • @mfulton26 Thanks! – sak Jul 27 '18 at 09:44
  • 2
    If there is a mutable variable in Kotlin that means it's required at some point. So, instead of being smartass propose a solution – Farid Jul 20 '19 at 07:15
  • 1
    @FARID who are referring to? – mfulton26 Jul 22 '19 at 16:24
  • 1
    to @GoodNightNerdPride, I forgot to tag the user – Farid Jul 22 '19 at 16:36
  • 1
    @FARID, I wasn't trying to be a smart ass. Re-thinking your design to rely less on mutability is a valid approach to solve concurrency issues. – Good Night Nerd Pride Jul 22 '19 at 17:10
  • 1
    @GoodNightNerdPride, apparently, I went far by calling you a name, sorry for that. But at some point, we have to use `mutable` consequently end up in a mess and looking for a proper solution. Every time I take a look at comments to see others' ideas on the solution while checking the answer. I just carried away for a moment by your answer and unnecessarily vent spleen on you )) – Farid Jul 23 '19 at 07:31
  • 1
    number 3's my favorite! – Alan Nelson Aug 14 '19 at 18:05
  • Will the first option be safe? I mean it will not create any NPE later right? @mfulton26. Because I will be making these changes in production environment. – Divya Gupta Dec 31 '19 at 07:56
  • 3
    Yes, it's safe. When a variable is declared as class global, any thread can modify its value. But in case of a local variable (a variable declared inside a function), that variable is not reachable from other threads, so safe to use. – Farid Dec 31 '19 at 09:13
  • 1
    @DivyaGupta, Farid is correct, it is null safe. See [Null Safety - Kotlin Programming Language](https://kotlinlang.org/docs/reference/null-safety.html) for more details. – mfulton26 Jan 02 '20 at 15:14
  • Doesn't adding local variables beat the purpose of DataBinding? I'm more confused. –  Jun 29 '20 at 22:07
  • @mfulton26, i know that it's and old post but i will be glad to understand what happens when *add* function get a *return* as an argument. I have never seen a think like this before – Eitanos30 Oct 14 '20 at 18:10
  • @Eitanos30, here `return` is used as an expression which means `add` never gets called if `left` is `null`; execution flow returns from whatever functions contains this statement. – mfulton26 Oct 15 '20 at 12:05
  • 1
    @mfulton26, thanks. Kotlin has so much sugar syntax that it make me crazy :( – Eitanos30 Oct 15 '20 at 12:08
  • @Eitanos30 It does drive people crazy but it's, actually, great, if not warned by IDE you could've simply overlooked it. – Farid Oct 20 '20 at 09:03
48

1) Also you can use lateinit If you sure do your initialization later on onCreate() or elsewhere.

Use this

lateinit var left: Node

Instead of this

var left: Node? = null

2) And there is other way that use !! end of variable when you use it like this

queue.add(left!!) // add !!
Radesh
  • 10,690
  • 3
  • 42
  • 60
  • what does it do? – c-an May 07 '19 at 01:34
  • @c-an it make your variable initialize as null but expect sure to initialize later in code. – Radesh May 07 '19 at 06:52
  • Then, isn't it the same? @Radesh – c-an May 07 '19 at 07:00
  • @c-an same with what? – Radesh May 07 '19 at 07:11
  • `lateinit var left: Node` and `var left: Node? = null` – c-an May 07 '19 at 11:30
  • 1
    i answered above question that is Smart cast to 'Node' is impossible, because 'left' is a mutable property that could have been changed by this time this code prevent that error by specify variable kind. so compiler no need for smart cast – Radesh May 07 '19 at 11:47
  • Although the answer could be helpful to some, don't forget that you're prohibiting the value of the variable to be `null` when you declare it as `lateinit var left: Node` – Farid Dec 31 '19 at 09:11
29

There is a fourth option in addition to the ones in mfulton26's answer.

By using the ?. operator it is possible to call methods as well as fields without dealing with let or using local variables.

Some code for context:

var factory: ServerSocketFactory = SSLServerSocketFactory.getDefault();
socket = factory.createServerSocket(port)
socket.close()//smartcast impossible
socket?.close()//Smartcast possible. And works when called

It works with methods, fields and all the other things I tried to get it to work.

So in order to solve the issue, instead of having to use manual casts or using local variables, you can use ?. to call the methods.

For reference, this was tested in Kotlin 1.1.4-3, but also tested in 1.1.51 and 1.1.60. There's no guarantee it works on other versions, it could be a new feature.

Using the ?. operator can't be used in your case since it's a passed variable that's the problem. The Elvis operator can be used as an alternative, and it's probably the one that requires the least amount of code. Instead of using continue though, return could also be used.

Using manual casting could also be an option, but this isn't null safe:

queue.add(left as Node);

Meaning if left has changed on a different thread, the program will crash.

Zoe
  • 23,712
  • 16
  • 99
  • 132
  • As far as I understand, the '?.' operator is checking if the variable on its left side is null.. In the example above it would be 'queue'. The error 'smart cast impossible' is referring to the parameter "left" being passed into the method "add"... I still get the error if I use this approach – FRR Nov 17 '17 at 16:06
  • Right, the error is on `left` and not `queue`. Need to check this, will edit the answer in a minute – Zoe Nov 17 '17 at 16:11
5

Change var left: Node? = null to lateinit var left: Node. Problem solved.

Mohammed mansoor
  • 565
  • 2
  • 10
  • 17
4

The practical reason why this doesn't work is not related to threads. The point is that node.left is effectively translated into node.getLeft().

This property getter might be defined as:

val left get() = if (Math.random() < 0.5) null else leftPtr

Therefore two calls might not return the same result.

Roland Illig
  • 37,193
  • 10
  • 75
  • 113
3

Your most elegant solution must be:

var left: Node? = null

fun show() {
    left?.also {
        queue.add( it )
    }
}

Then you don't have to define a new and unnecessary local variable, and you don't have any new assertions or casts (which are not DRY). Other scope functions could also work so choose your favourite.

Simon Jacobs
  • 341
  • 2
  • 5
2

Do this:

var left: Node? = null

fun show() {
     val left = left
     if (left != null) {
         queue.add(left) // safe cast succeeds
     }
}

Which seems to be the first option provided by the accepted answer, but that's what you're looking for.

EpicPandaForce
  • 71,034
  • 25
  • 221
  • 371
1

For there to be a Smart Cast of the properties, the data type of the property must be the class that contains the method or behavior that you want to access and NOT that the property is of the type of the super class.


e.g on Android

Be:

class MyVM : ViewModel() {
    fun onClick() {}
}

Solution:

From: private lateinit var viewModel: ViewModel
To: private lateinit var viewModel: MyVM

Usage:

viewModel = ViewModelProvider(this)[MyVM::class.java]
viewModel.onClick {}

GL

Braian Coronel
  • 17,823
  • 4
  • 30
  • 33
0

Try using the not-null assertion operator...

queue.add(left!!) 
Vadim Kotov
  • 7,103
  • 8
  • 44
  • 57
Bikeboy
  • 503
  • 4
  • 5
  • 3
    Dangerous. For the same reason the auto-casting doesn't work. – Jacob Zimmerman Jan 24 '19 at 20:48
  • 3
    It may cause app crash if left is null. – Pritam Karmakar Apr 22 '19 at 04:01
  • How can it be null? Code in question checks it: "if (left != null)". If the value could change to null it could change to another value, too. The app would crash but the results of the method could be unexpected. – The incredible Jan Nov 04 '20 at 10:15
  • Literally, the error given by Kotlin (in the title of the question) tells you that it could have changed. Either you are smarter than the compiler (rarely the case) or this is possible when there are multiple threads running (which always is the case in Android, you at least have a UI thread + several worker threads). – Albert Vaca Cintora Dec 23 '20 at 14:30
0

How I would write it:

var left: Node? = null

fun show() {
     val left = left ?: return
     queue.add(left) // no error because we return if it is null
}
tonisives
  • 905
  • 9
  • 14