RecyclerView with CheckBox does not keep selected items

I have a list with RecyclerView + Adapter + ViewHolder and each item list contains a description and a CheckBox. If I select an item and scroll down the list RecyclerView does not keep the selection when I scroll up again.

Here is my code:

class MainActivity : AppCompatActivity() {

    private lateinit var binding: ActivityMainBinding
    private lateinit var adapter: MyAdapter
    private val items = mutableListOf<Item>()

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        binding = ActivityMainBinding.inflate(layoutInflater)
        setContentView(binding.root)

        adapter = MyAdapter()
        binding.recyclerView.adapter = adapter
    }

    override fun onResume() {
        super.onResume()

        for (i in 1..100) {
            items.add(Item("Item $i", false))
        }
    }

    inner class MyAdapter : RecyclerView.Adapter<MyViewHolder>() {

        override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): MyViewHolder {
            val view = LayoutInflater.from(parent.context).inflate(R.layout.item_layout, parent, false)
            return MyViewHolder(view)
        }

        override fun onBindViewHolder(holder: MyViewHolder, position: Int) {
            val item = items[position]
            holder.textView.text = item.description
            holder.checkBox.isChecked = item.isSelected
            holder.checkBox.setOnCheckedChangeListener {_, isChecked ->
                item.isSelected = isChecked
                holder.checkBox.isChecked = isChecked
            }
        }

        override fun getItemCount(): Int {
            return items.size
        }
    }

    inner class MyViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {

        val textView: TextView = itemView.findViewById(R.id.textView)
        val checkBox: CheckBox = itemView.findViewById(R.id.checkBox)
    }

    data class Item(val description: String, var isSelected: Boolean)
}

I have a theory about what's going wrong, but I didn't test it. It has to do with this code:

holder.checkBox.isChecked = item.isSelected
holder.checkBox.setOnCheckedChangeListener {_, isChecked ->
    item.isSelected = isChecked
    holder.checkBox.isChecked = isChecked
}

The first time you bind a view holder, it has this change listener added to it. The change listener is capturing a reference to the current item.

When the view holder scrolls off the screen and then back onto the screen, the old listener is still attached and capturing the old item. So when you call holder.checkBox.isChecked = item.isSelected, you might be triggering the old listener, which is mutating the old item to the wrong state.

So to fix this, set your listener before you update the check box state. You can also remove the useless holder.checkBox.isChecked = isChecked line. The check box state is automatically updated to the new state before the listener is called.

Fixed version of above code:

holder.checkBox.setOnCheckedChangeListener {_, isChecked ->
    item.isSelected = isChecked
}
holder.checkBox.isChecked = item.isSelected

An alternate solution is to set your click listener only once when the ViewHolder is created. This is more efficient anyway, because it doesn't have to allocate a new listener every time a view scrolls onto the screen. To do this, we find the current item at the time the listener is invoked by using absoluteAdapterPosition. Then it isn't capturing a specific item, so the same listener is usable at any time.

inner class MyViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {

    val textView: TextView = itemView.findViewById(R.id.textView)
    val checkBox: CheckBox = itemView.findViewById(R.id.checkBox).apply {
        setOnCheckedChangeListner { _, isChecked ->
            items[absoluteAdapterPosition].isSelected = isChecked
        }
    }
}