DiffUtil Not working in nested recyclerview Kotlin

Question

I have two recycler views. My view is not updated until I used notifyDataSetChanged. I asked for a similar type of issue, but this time I have Github Link. So please have a look and explain to me what I am doing wrong. Thanks

MainActivity.kt

package com.example.diffutilexample

import android.os.Bundle
import android.util.Log
import androidx.activity.viewModels
import androidx.appcompat.app.AppCompatActivity
import com.example.diffutilexample.databinding.ActivityMainBinding

class MainActivity : AppCompatActivity() {

    private val viewModel by viewModels<ActivityViewModel>()
    private lateinit var binding: ActivityMainBinding
    private var groupAdapter: GroupAdapter? = null

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

        binding.button.setOnClickListener {
            viewModel.addData()
        }
    }

    private fun setupViewModel() {
        viewModel.groupListLiveData.observe(this) {
            if (groupAdapter == null) {
                groupAdapter = GroupAdapter()
                binding.recyclerview.adapter = groupAdapter
            }
            groupAdapter?.submitList(viewModel.groupList?.toMutableList())
            binding.recyclerview.post {
                groupAdapter?.notifyDataSetChanged()
            }
        }
    }
}

ActivityViewModel.kt

package com.example.diffutilexample

import android.app.Application
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.launch

class ActivityViewModel(app: Application) : AndroidViewModel(app) {

    var groupListLiveData: MutableLiveData<Boolean> = MutableLiveData()
    var groupList: ArrayDeque<Group>? = null
        set(value) {
            field = value
            groupListLiveData.postValue(true)
        }
    var value = 0

    fun fetchData() {
        viewModelScope.launch {
            val response = ApiInterface.create().getResponse()

            groupList = groupByData(response.abc)
        }
    }

    private fun groupByData(abc: List<Abc>?): ArrayDeque<Group> {
        val result: ArrayDeque<Group> = groupList ?: ArrayDeque()

        abc?.iterator()?.forEach { item ->
            val key = GroupKey(item.qwe)
            result.addFirst(Group(key, mutableListOf(item)))
        }
        return result
    }

    fun addData() {
        groupList?.let { lastList ->
            val qwe = Qwe("Vivek ${value++}", "Modi")
            val item = Abc(type = "Type 1", "Adding Message", qwe)
            val lastGroup = lastList[0]
            lastGroup.list.add(item)
            groupList = lastList
        }
    }
}

Please find the whole code in Github Link. I attached in above

Answer 1

I'm not entirely sure, and I admit I haven't extensively studied your code, and this is not a solution, but this might point you in the right direction of how to solve it.

The thing about

groupAdapter?.submitList(viewModel.groupList?.toMutableList())

Is that toMutableList() does indeed make a copy of the list. But each of the objects in the list are not copies. If you add things to an object in the original list, like you do in addData() it in fact is also already added to the copy that is in the adapter. That's why a new submitList doesn't recognize it as a change because it is actually the same as it was before the submitList.

As far as I understand, working with DiffUtil works best if the list you submit only contains objects that are immutable, so mistakes like this can't happen. I have ran into a similar problem before and the solution is also not straightforward. In fact, I don't entirely remember how I solved it back then, but hopefully this pushes you in the right direction.

Answer 2

I haven't debugged this, but if you remove your overuse of MutableLists and vars, and simplify your LiveData, you will likely eliminate your bug. At the very least, it will help you track down the problem.

MutableLists and DiffUtil do not play well together!

For example, Group's list should be a read-only List:

data class Group(
    val key: GroupKey,
    val list: List<Abc?> = emptyList()
)

It's convoluted to have a LiveData that only reports if some other property is usable. Then you're dealing with nullability all over the place here and in the observer, so it becomes hard to tell when some code is going to be skipped or not from a null-safe call. I would change your LiveData to directly publish a read-only List. You can avoid nullable Lists by using emptyList() to also simplify code.

You can avoid publicly showing your interior workings with the ArrayDeque as well. And you are lazy loading the ArrayDeque unnecessarily, which leads to having to deal with nullability unnecessarily.

class ActivityViewModel(app: Application) : AndroidViewModel(app) {

    private val _groupList = MutableLiveData<List<Group>>()
    val groupList: LiveData<List<Group>> get() = _groupList
    private val trackedGroups = ArrayDeque<Group>()
    private var counter = 0

    fun fetchData() {
        viewModelScope.launch {
            val response = ApiInterface.create().getResponse()
            addFetchedData(response.abc.orEmpty())
            _groupList.value = trackedGroups.toList() // new copy for observers
        }
    }

    private fun addFetchedData(abcList: List<Abc>) {
        for (item in abcList) {
            val key = GroupKey(item.qwe)
            trackedGroups.addFirst(Group(key, listOf(item)))
        }
    }

    fun addData() {
        if (trackedGroups.isEmpty())
            return // Might want to create a default instead of doing nothing?
        val qwe = Qwe("Vivek ${counter++}", "Modi")
        val item = Abc(type = "Type 1", "Adding Message", qwe)
        val group = trackedGroups[0]
        trackedGroups[0] = group.copy(list = group.list + item)

        _groupList.value = trackedGroups.toList() // new copy for observers
    }
}

In your Activity, since your GroupAdapter has no dependencies, you can instantiate it at the call site to avoid dealing with lazy loading it. And you can set it to the RecyclerView in onCreate() immediately.

Because of the changes in ViewModel, observing becomes very simple.

If you do something in setupViewModel() that updates a view immediately, you'll have a crash, so you should move it after calling setContentView().

class MainActivity : AppCompatActivity() {

    private val viewModel by viewModels<ActivityViewModel>()
    private lateinit var binding: ActivityMainBinding
    private val groupAdapter = GroupAdapter()

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        binding = ActivityMainBinding.inflate(layoutInflater).apply {
            setContentView(root)
            recyclerview.adapter = groupAdapter
            button.setOnClickListener {
                viewModel.addData()
            }
        }

        setupViewModel()
        viewModel.fetchData()
    }

    private fun setupViewModel() {
        viewModel.groupList.observe(this) {
            groupAdapter.submitList(it)
        }
    }
}

Your DiffUtil.ItemCallback.areItemsTheSame in GroupAdapter is incorrect. You are only supposed to check if they represent the same item, not if their contents are the same, so it should not be comparing lists.

override fun areItemsTheSame(oldItem: Group, newItem: Group): Boolean {
    return oldItem.key == newItem.key
}

And in GroupViewHolder, you are creating a new adapter for the inner RecyclerView every time it is rebound. That defeats the purpose of using RecyclerView at all. You should only create the adapter once.

I am predicting that the change in the nested list is going to look weird when the view is being recycled rather than just updated, because it will animate the change from what was in the view previously, which could be from a different item. So we should probably track the old item key and avoid the animation if the new key doesn't match. I think this can be done in the submitList() callback parameter to run after the list contents have been updated in the adapter by calling notifyDataSetChanged(), but I haven't tested it.

class GroupViewHolder(val binding: ItemLayoutBinding) : RecyclerView.ViewHolder(binding.root) {
    
    companion object {
        //...
    }

    private val adapter = NestedGroupAdapter().also {
        binding.nestedRecyclerview.adapter = it
    }

    private var previousKey: GroupKey? = null

    fun bindItem(item: Group?) {
        val skipAnimation = item?.key != previousKey
        previousKey = item?.key
        adapter.submitList(item?.list.orEmpty()) {
            if (skipAnimation) adapter.notifyDataSetChanged()
        }
    }
}

Side note: your adapters' bindView functions are confusingly named. I would just make those into secondary constructors and you can make the primary constructor private.

class GroupViewHolder private constructor(private val binding: ItemLayoutBinding) :
    RecyclerView.ViewHolder(binding.root) {

    constructor(parent: ViewGroup) : this(
        ItemLayoutBinding.inflate(
            LayoutInflater.from(parent.context),
            parent,
            false
        )
    )

    //...
}