ClementCardonnel / GhibliMovies

An app that presents a list of movies fetched from GhibliAPI.
MIT License
0 stars 0 forks source link

"Inconsistent associations for moves" #1

Open SquirrelMobile opened 3 years ago

SquirrelMobile commented 3 years ago

In MoviesViewController.swift

lines 183 : dataSource.apply(snapshot, animatingDifferences: true)

When we put the first and the second then the fourth movies in the favorites, it crashes.

sharedRoutine commented 3 years ago

In MoviesViewController.swift

lines 183 : dataSource.apply(snapshot, animatingDifferences: true)

When we put the first and the second then the fourth movies in the favorites, it crashes.

I just ran this code and it doesn't happen to me. I want to debug this exact message in another project and therefore I am trying to see when this happens. Are you still able to reproduce it and if so, how exactly with which device type, simulator or real device and which iOS Version?

ClementCardonnel commented 3 years ago

Hello @sharedRoutine, I had this issue during development, but couldn't reproduce it after a fix. Some people still experience it though. I don't know exactly how to make it appear consistently. Still, I remember clearly seing it using iOS 14.1/14.2 iPad simulator. If you want to have better luck reproducing the issue, you can try checking out a previous commit such as 1c5a326. Maybe it will be easier without my fix attempt. 🙂

Anyway, best of luck to you.

sharedRoutine commented 3 years ago

Hi @Pomme2Poule, thanks for this suggestion. I used an iPad Simulator on this commit hash and I made it crash! (I was never that happy to crash an app before). I think I fully understand this issue now and I managed to fix it in this commit hash (I haven't taken a look at your suggestion, but I think I know where the problem lies).

Let's take a look at commit hash 1c5a326. I managed to crash it by adding several movies to favorites, then toggling favorites and removing one of the favorites from the list. The first quick fix I thought about is calling performQuery with the onlyShowFavorites argument properly (since it has been forgotten) - so it switches from favorites to non favorites when deleting a film from favorites. But I wanted to fix it properly, so I reverted this change and let it crash again in order to look at the problem.

In this version the Film struct implement Hashable and Equatable. Two films are equal, when their id is equal - which is not how Equatable should be implemented. Equality is based on the properties, not the identification. The hash value is made by hashing the individual properties of the Film struct - here it would be fine to just hash the identifier. The problem specifically lies at the isFavorites property being hashed.

Let's take a look at what happens when you delete a movie: The old dataset (current) contains a film with identifier "1234" where isFavorite is true (resulting in a different hash) and the new data set contains a film with identifier "1234" but isFavorite is false. So when the diffing happens, it detects a deletion and insertion (given the missing hash value from before and a new hash value from new data), however the struct remains the same (equality) - which is an invalid move.

You can make it not crash by removing the isFavorite property from the hash(into hasher: inout Hasher) function or to be completely save, only hash the identifier and reimplement the == function to use all properties except the identifier.

So Hashable acts as an "is this element still in there" and then Equatable acts as "has it changed?"