Calling notifyDataSetChanged and updating the data, but RecyclerViewer still takes in old data

Question

I'm currently experimenting with a RecyclerViewer, but stumbled upon a problem: When I update the data and call notifyDataSetChanged, the RecyclerViewer updates it's view, but not with the new data, but rather with the old data.

I've searched through Stackoverflow for the problem, but in most cases the problem is that they either created two instances of the adapter (reference) or that they don't have a layout manager, and I believe that neither of those is my problem.

Here is my code for creating and updating the RecyclerViewer in the fragment in which it's hosted:

override fun onCreateView(
        inflater: LayoutInflater, container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View? {
        val view = inflater.inflate(R.layout.fragment_player_list_list, container, false)
        data = getPlayersAsList(requireContext(), gameUUID)
        if(data.isEmpty()){
            data = listOf(SharedPreferencesManager.Companion.Player("John", false))
        }
        // Set the adapter
        if(view is LinearLayout){
            view.children.forEach {
                if (it is RecyclerView) {
                    with(it) {
                        layoutManager = when {
                            columnCount <= 1 -> LinearLayoutManager(context)
                            else -> GridLayoutManager(context, columnCount)
                        }
                        Log.i("DEBUG", "The first adapter was called")
                        adapter = MyItemRecyclerViewAdapter(data)
                    }
                }
            }
        }

fun notifyDataUpdate(position: Int? = null) {
        if(view is LinearLayout){
            (view as LinearLayout).children.forEach {
                if(it is RecyclerView){
                    data = getPlayersAsList(requireContext(), gameUUID)
                    Log.i("DATA UPDATE", "Player list is now $data")
                    it.adapter?.notifyDataSetChanged()
                }
            }
        }
    }

And here is the code of the adapter:

import android.util.Log
import androidx.recyclerview.widget.RecyclerView
import android.view.LayoutInflater
import android.view.ViewGroup
import android.widget.TextView

import com.chuaat.hideandseek.databinding.FragmentPlayerListBinding
import com.google.android.material.button.MaterialButton
import java.util.*

/**
 * [RecyclerView.Adapter] that can display a [PlaceholderItem].
 * TODO: Replace the implementation with code for your data type.
 */
class MyItemRecyclerViewAdapter(
    private val values: List
) : RecyclerView.Adapter() {

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {

        return ViewHolder(
            FragmentPlayerListBinding.inflate(
                LayoutInflater.from(parent.context),
                parent,
                false
            )
        )

    }

    override fun onBindViewHolder(holder: ViewHolder, position: Int) {
        val item = values[position]
        Log.i("RECYCLER VIEWER", "SETTING VALUE OF $item")
        if(item.isSeeker){
            holder.buttonView.setIconResource(R.drawable.ic_baseline_search_24)
        }
        else{
            holder.buttonView.setIconResource(R.drawable.ic_outline_visibility_off_24)
        }
        holder.contentView.text = item.name
    }

    override fun getItemCount(): Int = values.size

    inner class ViewHolder(binding: FragmentPlayerListBinding) :
        RecyclerView.ViewHolder(binding.root) {
        val buttonView: MaterialButton = binding.toggleSeekerButton as MaterialButton
        val contentView: TextView = binding.content

        override fun toString(): String {
            return super.toString() + " '" + contentView.text + "'"
        }
    }

}

The log output I get when calling notifiyDataUpdate is:

I/DATA UPDATE: Player list is now [Player(name=Joe, isSeeker=false)]
I/RECYCLER VIEWER: SETTING VALUE OF Player(name=John, isSeeker=false)

As you can see the updated data is with a Player named Joe, but in onBindViewHolder the only value is the default Player ("John").

What is the problem I'm missing?

user_13598222 · Answer

You're not actually updating the data in your adapter

Here's how you initialise it:

data = getPlayersAsList(requireContext(), gameUUID)
if(data.isEmpty()){
    data = listOf(SharedPreferencesManager.Companion.Player("John", false))
}
...
adapter = MyItemRecyclerViewAdapter(data)

and that parameter in your adapter's constructor is your data source for the RecyclerView

class MyItemRecyclerViewAdapter(
    private val values: List

At this point, your Fragment has a list called data which contains your current data, and the Adapter has a reference to that same list. They're both looking at the same object - let's call it list 1.

Then you update your data in the Fragment, and notify the Adapter:

data = getPlayersAsList(requireContext(), gameUUID)
it.adapter?.notifyDataSetChanged()

But what you've done is create a new list, list 2, with that getPlayersAsList call. You assign that to data. So data points to list 2, the new data - but values in your adapter still points to list 1, the old list. So for the adapter, nothing's changed! It can't see the new data, so even though you notify it, it will still look the same.

You have two options here. Firstly, since you're already using this shared list that the Fragment and Adapter are both looking at, you can just update that list - which is what you should be doing if they're both sharing it, right?

// clear the old data
data.clear()
// replace it with the new items
// this is the same as data.addAll(getPlayersAsList(...))
data += getPlayersAsList(requireContext(), gameUUID)
// now the list the adapter is using has been updated, you can notify it
it.adapter?.notifyDataSetChanged()

This way you're updating the actual list the adapter uses as its dataset, so you'll see the changes when it refreshes.

The second way, and the one I'd recommend, is to completely separate the Adapter's data from anything the Fragment is holding onto. Having a shared list like this can be a source of bugs, where one component changes it in the background, affecting the state of another component. If you ever use a DiffUtil in a RecyclerView for example, mutating the current list will stop it from working, because it won't be able to compare for changes.

You could make the values property a public var and update that externally, then notify the adapter - but honestly, it's better to let the Adapter handle those details internally. A setter function is a lot cleaner to me:

// in the Adapter
private var values = emptyList()

fun setData(items: List) {
    // as a safety measure, creating a new list like this *ensures* that if the one
    // that was passed in is mutated, this internal one won't change. (The items
    // in the list can still be mutated of course!)
    values = items.toList()
    // now the -adapter- can decide on how/if it should update, based on its own
    // internal state and the new data. The Fragment shouldn't be concerned with those details
    notifyDataSetChanged()
}

Then when you want to update the adapter, just pass it the new data list:

// assuming you still want to keep a local reference to this data (if you don't need it, don't!)
data = getPlayersAsList(requireContext(), gameUUID)
// I'd really recommend just keeping a reference to your adapter when you create it,
// so you don't need to go searching for it and casting it like this
(it.adapter as? MyItemRecyclerViewAdapter)?.setData(data)

To initialise the adapter, you can either use this method:

// seriously, just store this in a `lateinit var adapter: MyItemRecyclerViewAdapter`
adapter = MyItemRecyclerViewAdapter()
adapter.setData(data)
recyclerView.adapter = adapter

Or you could keep the constructor parameter (which we're not using as a property to store the data, remember!) and use it to call the setData function:

class MyItemRecyclerViewAdapter(
    data: List // no val
) : RecyclerView.Adapter() {

    init {
        setData(data)
    }

And just as a hint - the way you're accessing your RecyclerView is complicated and not how you generally do things in Android. Give it an id in your layout XML file (R.layout.fragment_player_list_list) and then just do

val recyclerView = view.findViewById(R.id.whatever)

That's it! No need to loop through the hierarchy searching for it. If you store your Adapter in a variable, you probably won't need to touch the RV itself after setting it up - just call setData on your adapter reference

Calling notifyDataSetChanged and updating the data, but RecyclerViewer still takes in old data

1 Answers