search

luizgrp / SectionedRecyclerViewAdapter

An Adapter that allows a RecyclerView to be split into Sections with headers and/or footers. Each Section can have its state controlled individually.
MIT License
1.68k stars 372 forks source link

Is synchronized (sections) required in getCopyOfSectionsMap #104

Closed yccheok closed 6 years ago

yccheok commented 6 years ago

https://github.com/luizgrp/SectionedRecyclerViewAdapter/blob/1f248435ea09561c42afabf492adb427e2ab626e/library/src/main/java/io/github/luizgrp/sectionedrecyclerviewadapter/SectionedRecyclerViewAdapter.java#L519

Is synchronized (sections) { required, as it seems like unable to prevent java.util.ConcurrentModificationException, if user is calling getCopyOfSectionsMap from a non-UI thread.

I don't think it is a bad thing, if such exception is being thrown if user is calling getCopyOfSectionsMap from a non-UI thread.

luizgrp commented 6 years ago

Hi @yccheok! The intention of having the synchronized in this method is to avoid items being added to the map while the copy is being made. Do you think the responsibility of locking it should be of the user of lib?

yccheok commented 6 years ago

Hi @luizgrp

Yes. All the time, my view on this library is that, it shouldn't have any knowledge on threading. Hence, seeing synchronized keyword in the code, seem a bit awkward to me :)

I believe you had achieve goal of SectionedRecyclerViewAdapter - provides section liked UI effect for RecyclerView. For threading issues, it should be the responsible of another library, or another class, or responsible of developers themselves.

luizgrp commented 6 years ago

Cool, I agree with you on this, will change it!