Alternative to sectionAdapter.notifyDataSetChanged

[yccheok]

Most of the time, whenever there are background threads performing

• Change Section state from LOADING to EMPTY
• Change Section state from LOADED to EMPTY
• setHasHeader(true)/ setHasHeader(false)
• section.setVisible(true)/ section.setVisible(false)

I will use sectionAdapter.notifyDataSetChanged, so that visualize effect will take in immediately.

However, when I implement a complex drag-n-drop/ drag-n-move UX in one of the Sections, I wish to avoid using sectionAdapter.notifyDataSetChanged. The reason is that, if other Section threads call sectionAdapter.notifyDataSetChanged, it will break my Watchlist section drag-n-drop/ drag-n-move UX effect.

After experimenting, I had came out with the following code to replace sectionAdapter.notifyDataSetChanged

if (orders.isEmpty()) {
    orderSection.setState(Section.State.EMPTY);

    if (orderSection.hasHeader()) {
        int headerPosition = sectionAdapter.getHeaderPositionInAdapter(orderSection);
        orderSection.setHasHeader(false);
        sectionAdapter.notifyItemRemovedFromSection(orderSection, headerPosition);
    }

    // I think this works fine, if it changed from LOADING TO EMPTY. But,
    // what if it changed from LOADED (More than 1 item) to EMPTY? Does the
    // following code still work fine?
    sectionAdapter.notifyItemRangeChangedInSection(
            orderSection,
            0,
            1
    );
} else {
    orderSection.updateOrders(orders);
    if (!orderSection.hasHeader()) {
        orderSection.setHasHeader(true);
        sectionAdapter.notifyHeaderChangedInSection(orderSection);
    }

    if (orderSection.getState() != Section.State.LOADED) {
        orderSection.setState(Section.State.LOADED);
    }

    sectionAdapter.notifyItemRangeChangedInSection(
            orderSection,
            0,
            orderSection.getContentItemsTotal()
    );
}

1. When I hide header, I use notifyItemRemovedFromSection
2. When I show header, I use notifyHeaderChangedInSection
3. When I change State from LOADING to EMPTY, I use notifyItemRangeChangedInSection, with item count is 1.
4. When I change State from X to LOADED, I use notifyItemRangeChangedInSection, with item count is Section.getContentItemsTotal().

Do you think the above is a good way, so that our update operation is capped at Section level, and doesn't spread to other Section?

What other notification way I can use, if I have the following case?

1. When section.setVisible(true)/ section.setVisible(false)
2. When I change State from LOADED to EMPTY ?

Thanks.

[luizgrp]

1. When I hide header, I use notifyItemRemovedFromSection

You should call getHeaderPositionInAdapter before hiding the header and store the position in a variable, then hide it and call notifyItemRemoved with the position.

2. When I show header, I use notifyHeaderChangedInSection

You should call getSectionPosition before showing the header and store the position in a variable, then show it and call notifyItemInserted with the position.

We could add new helper methods for the scenarios when the visibility of the header or footer is changed:

notifyHeaderInsertedInSection

• Get position with getHeaderPositionInAdapter and call callSuperNotifyItemInserted

notifyFooterInsertedInSection

• Get position with getFooterPositionInAdapter and callSuperNotifyItemInserted

notifyHeaderRemovedInSection

• Get position with getSectionPosition and call callSuperNotifyItemInserted

notifyFooterRemovedInSection

• Get position with getSectionPosition + section.getSectionItemsTotal() - 1 and call callSuperNotifyItemRemoved

[luizgrp]

3. When I change State from LOADING to EMPTY, I use notifyItemRangeChangedInSection, with item count is 1.

You should call getSectionPosition then call notifyItemChanged with the position.

4. When I change State from X to LOADED, I use notifyItemRangeChangedInSection, with item count is Section.getContentItemsTotal().

You should call notifyItemChangedInSection with position 0. Then you should call notifyItemRangeInsertedInSection with the range 1 to getContentItemsTotal - 1. The reason is because for the RecyclerView, the view the for X state became the first item of the section, so we should notify the "change". Then we need to notify that items were inserted from the second item until the total minus the first item.

We can create helper methods:

notifyNotLoadedStateChanged(section, previousState)

• throw exception if state didn't change
• throw exception if previousState is LOADED
• throw exception if section state is LOADED
• do as explained above;

notifyStateChangedToLoaded(section, previousState)

• throw exception if state didn't change
• throw exception if section state isn't LOADED
• do as explained above;

notifyStateChangedFromLoaded(section, previousItemCount)

• throw exception if state is LOADED
• call notifyItemRangeRemovedInSection with the range 1 to previousItemCount
• call notifyItemChangedInSection with position 0

[luizgrp]

What other notification way I can use, if I have the following case?

1. When section.setVisible(true)/ section.setVisible(false)
2. When I change State from LOADED to EMPTY ?

I believe these questions were answered in the comments above.

Let me know if everything worked fine with the changes suggested.

[yccheok]

Thanks for the comprehensive information! That's really helpful.

I will fork and make change to our own project usage. If I manage to write unit test for the changes, I will send PR. If not, I will send PR next time :)

One quick note on

private void notifyStateChangedToLoaded(Section section) {
    sectionAdapter.notifyItemChangedInSection(section, 0);
    int count = section.getContentItemsTotal() - 1;
    if (count > 0) {
        sectionAdapter.notifyItemRangeChangedInSection(section, 1, count);
    }
}

I can't see how it is different from

private void notifyStateChangedToLoaded(Section section) {
    sectionAdapter.notifyItemRangeChangedInSection(section, 0, section.getContentItemsTotal());
}

as both seem like cover same range of rows. Do you mind to explain more?

[luizgrp]

It was a typo in my comment, just edited it, it should be notifyItemRangeInsertedInSection instead of notifyItemRangeChangedInSection.

// your code modified
private void notifyStateChangedToLoaded(Section section) {
    sectionAdapter.notifyItemChangedInSection(section, 0);
    int count = section.getContentItemsTotal() - 1;
    if (count > 0) {
        // changed the call below to notifyItemRangeInsertedInSection
        sectionAdapter.notifyItemRangeInsertedInSection(section, 1, count);
    }
}

[yccheok]

Thank you. That makes very much sense to me now.

[yccheok]

Sorry for late response. I had been working on other stuffs. Now, I will look into this issues seriously :)

Do you prefer this kind of code

public void notifyHeaderInsertedInSection(String tag) {
    int headerPosition = getHeaderPositionInAdapter(tag);

    callSuperNotifyItemInserted(headerPosition);
}

public void notifyHeaderInsertedInSection(Section section) {
    int headerPosition = getHeaderPositionInAdapter(section);

    callSuperNotifyItemInserted(headerPosition);
}

or

public void notifyHeaderInsertedInSection(String tag) {
    notifyHeaderInsertedInSection(this.getSection(tag));
}

public void notifyHeaderInsertedInSection(Section section) {
    int headerPosition = getHeaderPositionInAdapter(section);

    callSuperNotifyItemInserted(headerPosition);
}

[luizgrp]

no worries, no rush with this.

regarding the code, it's not a big deal, first one seems clearer to me 👍

[yccheok]

@luizgrp

I came across a problem when designing notification when there's visibility change on Section.

My initial code is

public void notifyVisibilityChangedToVisible(Section section) {
    int sectionPosition = getSectionPosition(section);

    int sectionItemsTotal = section.getSectionItemsTotal();

    callSuperNotifyItemRangeInserted(sectionPosition, sectionItemsTotal);
}

public void notifyVisibilityChangedToInvisible(Section section, int previousSectionPosition) {
    int sectionItemsTotal = section.getSectionItemsTotal();

    callSuperNotifyItemRangeInserted(previousSectionPosition, sectionItemsTotal);
}

However, I found such API is not user friendly. It is difficult to developer to keep track previousSectionPosition himself.

Is it possible to have something like this?

public void notifyVisibilityChangedToInvisible(Section section) {
    int sectionPosition = getSectionPositionEx(section);

    int sectionItemsTotal = section.getSectionItemsTotal();

    callSuperNotifyItemRangeInserted(previousSectionPosition, sectionItemsTotal);
}

public int getSectionPositionEx(Section section) {
    int currentPos = 0;

    for (Map.Entry<String, Section> entry : sections.entrySet()) {
        Section loopSection = entry.getValue();

        if (loopSection == section) {
            return currentPos;
        }

        // ignore invisible sections
        if (!loopSection.isVisible()) continue;

        int sectionTotal = loopSection.getSectionItemsTotal();

        currentPos += sectionTotal;
    }

    throw new IllegalArgumentException("Invalid section");
}

[luizgrp]

There is no mechanism in place for the adapter to keep track of the changes in the Section, we might be able to add a listener for the header/footer but not for the items, if we add a listener for the items we will break the flexibility of the adapter.

So in this case the dev has to keep track of the changes and notify the adapter accordingly. Why do you think the code below would be difficult?

int previousSectionPosition = adapter.getSectionPosition(section);
section.setVisible(false);
adapter.notifyVisibilityChangedToInvisible(section, previousSectionPosition);

[yccheok]

Fair enough. I will tend stick to our original plan. Do you think it is necessary to make the following utility functions part of the lib, or not?

void setInvisibleAndNotifyImmediately(Section section) {
    int previousSectionPosition = adapter.getSectionPosition(section);
    section.setVisible(false);
    adapter.notifyVisibilityChangedToInvisible(section, previousSectionPosition);
}

[luizgrp]

No, I'd rather not having any methods in the adapter that changes the state of the Section!

[yccheok]

Hi @luizgrp

Do we need unit test for "tag" version, if we already have unit test for "section" version?

https://github.com/luizgrp/SectionedRecyclerViewAdapter/pull/64/commits/67715a9610605c78a7a47a13b51aa0e3cd2fffbc

Also, not sure we can have a first round code review on https://github.com/luizgrp/SectionedRecyclerViewAdapter/pull/64 , as there are more functions to be added later, which will make the code review more cumbersome.

notifyVisibilityChangedToVisible
notifyVisibilityChangedToInvisible
notifyFooterInsertedInSection
notifyHeaderInsertedInSection

Thank you very much

[luizgrp]

create separate pull requests for the other functions that you add later 👍

I will review the need of unit test with the PR and let you know there

[yccheok]

@luizgrp

Thanks.

I had finished changes for this round. Do you mind to review it?

https://github.com/luizgrp/SectionedRecyclerViewAdapter/pull/65/files

[yccheok]

@luizgrp

Sorry. I'm not trying to rush you on reviewing or pushing to production. Just would like to know, whether you have any estimated time, on when PR will be finished reviewed and pushed to production?

Currently, we are using your lib heavily in near-to-go production environment. We have some bug fixes, which requires those newly added utilities functions from this lib.

If time are still required on your side, I will just make a temporary branch at my forked, and refer from my forked branch temporary.

No hurry, really! I fully understand, quality code requires longer baked time :)

Thank you very much for this high quality library. It really help a lot in our project.

[luizgrp]

No worries, thanks for your contributions! Let's aim for this Saturday afternoon?

[luizgrp]

Done in #65 and #66