ListAdapter DiffUtils newItem and oldItem the same when submitList() called

Question

Just FYI, I'm not exactly looking for a 'fix' but for an explanation and a discussion that might help understand a little bit more how seemingly silly things like these work.

I was working on this bigger project when I realized that somewhere, a certain list wasn't being updated correctly. Looking a little closer, the items, were correctly being modified, and if you 'scrolled away' and back, the item's information would be displayed correctly.

I stumbled upon this article: ListAdapter not updating item in RecyclerView

But the difference here, is that in fact, DiffUtils was being called, but somehow the newItem and oldItem were the same! I understand that the library assumes you are using Room or any other ORM which offers a new async list every time it gets updated, but here's the thing. If I submit the list "naively" DiffUtils is not even called. But, if I submit the list as list.toMutableList() like some suggest then, DiffUtils IS called, but somehow the items, new and old, are already the same, hence, nothing gets updated at that moment (verified this by placing breakpoints inside areContentsTheSame).

I leave you here the relevant snippets and a link to a test project I created just so I could encapsulate the behavior and test it separately from everything else.

The Fragment - just calling the submitList

viewModel.items.observe(viewLifecycleOwner) {
    adapter.submitList(it.toMutableList())
}

ViewModel

private val _items = MutableLiveData<List<SimpleItem>>()
val items: LiveData<List<SimpleItem>>
    get() = _items

init {
    _items.value = ItemsRepo.getItems()
}

fun onItemClick(itemId: Int) {
    ItemsRepo.addItemCount(itemId)
    _items.value = ItemsRepo.getItems()
}

The "Repo" I create some data object ItemsRepo {

private var items = mutableListOf(
    SimpleItem(1),
    SimpleItem(2),
    SimpleItem(3),
    SimpleItem(4),
    SimpleItem(5)
)

fun getItems(): List<SimpleItem> {
    return items
}

fun addItemCount(itemId: Int) {
    items.find { it.itemId == itemId }?.let {
        it.itemClickCount += 1
    }
}

The GitHub repo: https://github.com/ellasaro/ListAdapterTest

Cheers!

Linalinacre

Answer 1

Don't use mutable data classes or mutable lists with DiffUtil. It can lead to all kinds of problems. DiffUtil relies on comparing two lists, so if one of them is mutable and has been changed, it can't compare old and new successfully because there's no record of the previous state.

I didn't take the time to narrow down your exact issue, but I bet if you change your Repo's getItems() to return items.toList() (so mutating the Repo doesn't mutate downstream lists), and change SimpleItem to be an immutable class, your problems will go away.

Making SimpleItem immutable will be a little bit of hassle, unfortunately. The click listener instead of mutating the item will have to report back to the repo the id of the item that changed, and the repo must manually swap it out, and then you refresh the list.

It will be cleaner if your Repo returns a Flow of lists that automatically emits when changes are reported to it. Then your ViewModel doesn't have to both report changes and then remember to manually query the list state again.

I would use toList() and not toMutableList(). A mutable list communicates that you plan to mutate the list instead of just readding it, which you must never do with a list being passed to a DiffUtil.

Monkery

You are 100% right. I'd already tried this: if your Repo returns a Flow of lists that automatically emits when changes are reported and it worked, but I was trying to figure out why this other approach didn't. The problem indeed seemed to be the mutability of it all. Making SimpleItem's click count immutable, and returning a List from getList() worked perfectly. My Adapter's click handler was already returning the item's id so I searched the index of the element and swapped it with a modified copy. I now understand (sort of) the cause of the problem. Thanks for your insight! – Linalinacre

As an afterthought, when you create Room database entities, whether their properties are var or val this problems doesn't seem to happen. Any thoughts on that? It does seem to be a problem more of the mutability of the list than of the class. If I leave the itemClickCount property as var but replace the element altogether and submit the updated immutable list, it still works. Just trying to wrap my head around the different possible scenarios. – Linalinacre

Surely, every time Room generates a new result to return, it freshly parses the latest data into newly instantiated data class instances and puts them in a brand new List instance. The alternative would be to go through the complexity of seeing if those properties are mutable and if so, caching items that have been returned before and mutating them, just to create a huge code smell and ambiguous behavior. – Monkery

Answer 2

Declaring the itemClickCount property as val, and getting the list as an immutable list from the Repo object did the trick as Tenfour04 suggested.

As an additional observation, if I keep the itemClickCount property as var but replace the element altogether and re-submit the updated list, it works correctly. So the problem seems to be modifying the object's mutable property directly in the Repo's list. Using .toList() in getList() didn't help in that case.

Linalinacre

Answer 3

When I debugged my application, I saw that the equals function of the data class was called after the areContentsTheSame function.

If the data class does not override the equals method, oldItem and newItem will always be the same.

To solve this problem, simply override the equals function in the data class.

override fun equals(other: Any?): Boolean {
    return super.equals(other)
}

Startle

That sounds odd since data classes always pre-implement equals(), getters, setters, copy(), etc. Haver you come across cases where a data class hasn't implemented equals()? – Linalinacre

@MiguelLasa yes, I just tested it without the equals function in the data class and the areContentsTheSame function started to return true every time even when I updated my data. Could this be some kind of bug? – Startle