37

Why is Android Studio showing an error when I use script №2? I've found no difference between №1 and №2.

class Adapter {
    var nameList: ArrayList<String>? = null
}

class Program {
    private fun send() {
        val list: ArrayList<String> = ArrayList()
        val adapter = Adapter()

// Case 1
        var otherList = adapter.nameList
        if (otherList != null) {
            list.addAll(otherList) // <--- no error
        }

// Case 2
        if (adapter.nameList != null) {
            list.addAll(adapter.nameList) // <--- Error here
            // Smart cast to 'kotlin.collections.ArrayList<String> /* = java.util.ArrayList<String> */' is impossible, because 'adapter.nameList' is a mutable property that could have been changed by this time
        }
    }
}

Please explain this case.

bighugedev
  • 379
  • 1
  • 3
  • 11
quangkid
  • 1,287
  • 1
  • 12
  • 31
  • I added some recommended solutions for you below. – spierce7 Oct 12 '17 at 04:31
  • Possible duplicate of [Smart cast to 'Type' is impossible, because 'variable' is a mutable property that could have been changed by this time](https://stackoverflow.com/questions/44595529/smart-cast-to-type-is-impossible-because-variable-is-a-mutable-property-tha) – Marat Apr 01 '19 at 18:57

3 Answers3

45

The IDE should give you a warning, explaining that after the null check, it's possible that adapter.nameList was changed by another thread, and that when you call list.addAll(adapter.nameList), adapter.nameList could actually be null by that point (again, because a different thread could have changed the value. This would be a race condition).

You have a few solutions:

  1. Make nameList a val, which makes its reference final. Since it's final, it's guaranteed another thread can't change it. This probably doesn't fit your use case.

    class Adapter {
        val nameList : ArrayList<String>? = null
    }
    
  2. Create a local copy of name list before you do the check. Because it's a local copy, the compiler knows that another thread can't access it, and thus it can't be changed. The local copy could be defined with either a var or a val in this case, but I recommend val.

    val nameList = adapter.nameList
    if (nameList != null) {
        list.addAll(nameList)
    }
    
  3. Use one of the utility functions that Kotlin provides for just such a case as this. The let function copies the reference it's called on as a parameter using an inline function. This means that it effectively compiles down to be the same as #2, but it's a bit more terse. I prefer this solution.

    adapter.nameList?.let { list.addAll(it) }
    
sebnukem
  • 8,143
  • 6
  • 38
  • 48
spierce7
  • 14,797
  • 13
  • 65
  • 106
  • can we use **lateinit var** ? – Amit Vaghela Dec 28 '17 at 07:11
  • `lateinit var` are limited to member properties, meaning they are declared in the top portion of a class. Meaning it wouldn't be declared in the scope of any code you wrote, meaning it'd have the same problem. – spierce7 Dec 29 '17 at 15:56
  • How can we reassign a `val` type ? will it not throw an error? – ASN Aug 15 '18 at 03:10
  • 1
    You can't reassign a `val` type. The definition of a `val` type is that it is `final`, or that it can't be reassigned. – spierce7 Aug 15 '18 at 11:09
  • @ASN Consider a DTO containing annotated val properties. Even though the property is a val the annotation can affect the value of the backing field, introducing the uncertainty: Whether the property getter will return the same value or not. – NSaran Feb 03 '23 at 01:52
6

Your adapter.nameList is mutable property so please convert it to immutable.

Use this

  val nameList : ArrayList<String>? = null

Instead of this

  var nameList : ArrayList<String>? = null

Or you can also solve this problem by assert of non null Assert

            list.addAll(adapter.nameList!!)

Note :- !! is evaluated at runtime, it's just an operator.

The expression (x!!)

throws a KotlinNullPointerException if x == null, otherwise, it returns x cast to the corresponding non-nullable type (for example, it returns it as a String when called on a variable with type String?).

vishal jangid
  • 2,967
  • 16
  • 22
  • Your solution is good. But I want to know the reason why.@michael-anderson answer show the right point. – quangkid Oct 12 '17 at 04:28
  • 2
    This is bad practice. Using `!!` is really an indicator that you are probably doing things wrong. It's very rare that you should ever use `!!`. – spierce7 Oct 13 '17 at 00:06
  • yeah i used !!. even android studio also give same suggestion to use !! – vishal jangid Oct 13 '17 at 03:36
  • @vishaljangid If you are declaring above variable as `val nameList : ArrayList? = null`, isn't that always null? and it will crash – Bharatesh Oct 09 '18 at 09:58
2

adapter.nameList is a mutable property that could have been changed`

The reason for this check and error message is threads. What you have is called a race-condition. In many similar cases it is possible for another thread to change the value of adapter.namelist between the nullity check and the list.addAll call. Clearly this can not happen in your case as the adapter is not leaked from the send function, but I guess the compiler isn't smart enough to know that.

In contrast there is no race condition in case 1 as the namelist is only accessed once.

Also this can not happen if namelist is val rather than var - since the compiler then knows it can not change - so it can not change from non-null to null.

Abdelilah El Aissaoui
  • 4,204
  • 2
  • 27
  • 47
Michael Anderson
  • 70,661
  • 7
  • 134
  • 187