3

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!

Miguel Lasa
  • 470
  • 1
  • 7
  • 19

2 Answers2

3

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.

Tenfour04
  • 83,111
  • 11
  • 94
  • 154
  • 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! – Miguel Lasa Feb 05 '22 at 18:24
  • 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. – Miguel Lasa Feb 05 '22 at 19:50
  • 1
    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. – Tenfour04 Feb 05 '22 at 23:08
1

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.

Miguel Lasa
  • 470
  • 1
  • 7
  • 19