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

Some questions regarding removeSection(String tag) #84

Closed yccheok closed 6 years ago

yccheok commented 7 years ago

Recently, I plan to remove a Section from adapter for the first time.

However, I have few concern regarding the code.

public void removeSection(String tag) {
    this.sections.remove(tag);
}

public void addSection(String tag, Section section) {
    this.sections.put(tag, section);
    this.sectionViewTypeNumbers.put(tag, viewTypeCount);
    viewTypeCount += VIEW_TYPE_QTY;
}

May I know, during removing, besides removing item from this.sections, is there any reason why we don't perform removal from sectionViewTypeNumbers too?

Also, does it make sense if we provide

public void removeSection(Section section)

Thank you.

luizgrp commented 6 years ago

Hi @yccheok,

Nice catch! It should also remove from sectionViewTypeNumbers.

Regarding viewTypeCount, one option is not to decrement it, the other option is to decrement it and also loop through the entries in sectionViewTypeNumbers to decrement the values of the ones that were added after the section being removed.

But the current code should work fine without any side effects.

I will change it and add some tests. I will also add removeSection(Section).

Thanks!

yccheok commented 6 years ago

Thanks for planning to fix.

Yes. I understand that, not removing unused Section from sectionViewTypeNumbers, will not create any side-effect.

I feel Regarding viewTypeCount, one option is not to decrement it is better, as it is simpler, and reduce opportunity to introduce potential bug.

Btw, want to thank you for creating such library. We manage to use it in our application, to create many card view based screens. Here's how they looks like : https://plus.google.com/communities/116772191905350727072 Most screens which have card view based, are built on the top of SectionedRecyclerViewAdapter.

They work pretty solid so far. We are quite happy :)

luizgrp commented 6 years ago

Your app looks amazing! Congrats!

I will add a section to the README with apps that use the library and list your app there, if you don't mind?

yccheok commented 6 years ago

It is my great honour to be mentioned :)

By the way, recently I have some new idea on the builder pattern. Instead of

new SectionParameters.Builder

can we have static factory method like

SectionParameters.builder

Less typing and looks cleaner :)