search

TradeMe / MapMe

The Android maps adapter
MIT License
841 stars 75 forks source link

Double notifyDataSetChanged may cause crash #17

Closed Gloix closed 6 years ago

Gloix commented 6 years ago

I don't know if it's a bad use of the library or not, but when performing the following sequence of actions, bad things can occur.

  1. Create a list with one item and create an adapter with it
  2. Call notifyDataSetChanged(). After that, getItemCount() gets called, returning 1 as result.
  3. Immediately, and before the adapter is able to refresh its annotations, remove the single item from the list
  4. Call notifyDataSetChanged(). After that, getItemCount() gets called, returning 0 as result.
  5. In the next view refresh, the adapter calls onCreateAnnotation(...) with position=0, leading to a crash since there are no items in the list.

Apparently, the adapter remembers the first amount of annotations to be added to the map, but the second call to notifyDataSetChanged() does not get taken into account.

Gloix commented 6 years ago

Maybe an updates.clear() inside notifyDataSetChanged() could solve it(?). Unfortunately I cannot quick-fix in my project it since it's a private member 😕.

josh-burton commented 6 years ago

Would you be able to create a sample project that demonstrates the crash?

Gloix commented 6 years ago

Sure, here it is https://github.com/Gloix/MapMeBug Remember to use a valid Google Maps Key to test it. Also, note that I don't even have to modify the list of adapted items to trigger the error. I just need to call notifyDataSetChanged() twice.

josh-burton commented 6 years ago

Great, thanks for this!

It seems like the fix will be quite simple although it will require a bit of testing.