Propasal on notifyDataSetChangedInSection

[yccheok]

Followed up from https://github.com/luizgrp/SectionedRecyclerViewAdapter/issues/60 and snapshot in https://github.com/luizgrp/SectionedRecyclerViewAdapter/tree/adfe80e60fce236f2309062f9fb151e32264a6fe , we start to use it heavily in our production app.

However, we notice we need to call different notify functions based on different situation.

from LOADING to LOADED from LOADED to EMPTY from LOADING to EMPTY from LOADED to LOADED (new rows added, or same row but content changed, ...) ...

Hence, at the end, developers feel headache on which notify functions to be called.

We build this utility function to reduce such headache.

public static void notifyDataSetChangedInSection(
        SectionedRecyclerViewAdapter sectionedRecyclerViewAdapter,
        Section section,
        Section.State previousState,
        int previousContentItemsCount
) {
    final Section.State state = section.getState();

    if (previousState == state) {
        int contentItemsTotal = section.getContentItemsTotal();

        if (contentItemsTotal == previousContentItemsCount) {
            sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                    section,
                    0,
                    contentItemsTotal
            );
        } else if (contentItemsTotal > previousContentItemsCount) {
            sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                    section,
                    0,
                    previousContentItemsCount
            );  

            int count = contentItemsTotal - previousContentItemsCount;

            sectionedRecyclerViewAdapter.notifyItemRangeInsertedInSection(
                    section,
                    previousContentItemsCount,
                    count
            );
        } else if (contentItemsTotal < previousContentItemsCount) {
            sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                    section,
                    0,
                    contentItemsTotal
            );

            int count = previousContentItemsCount - contentItemsTotal;

            sectionedRecyclerViewAdapter.notifyItemRangeRemovedFromSection(
                    section,
                    contentItemsTotal,
                    count
            );
        }
    } else {
        if (state == Section.State.LOADED) {
            sectionedRecyclerViewAdapter.notifyStateChangedToLoaded(section, previousState);
        } else if (previousState == Section.State.LOADED) {
            sectionedRecyclerViewAdapter.notifyStateChangedFromLoaded(section, previousContentItemsCount);
        } else {
            sectionedRecyclerViewAdapter.notifyNotLoadedStateChanged(section, previousState);
        }
    }
}

1. May I know, is our implementation correct? Is there any mistake in our implementation?

2. Is there any way to make the usage of this function simpler? i.e., less input parameters required. How we wish to have notifyDataSetChangedInSection(section) 💯 (I can understand the difficulty of having such function)

3. Are you interested to have notifyDataSetChangedInSection in your lib?

Thank you.

[luizgrp]

Not sure it's possible to the adapter to predict what happened to the dataset based on the current/previous content item count and state, only developer can tell what has changed.

For example, In the third comparison (contentItemsTotal > previousContentItemsCount) it is calling notifyItemRangeChangedInSection but if some items were inserted or removed in that range, the animation will be wrong, it will look like the items were updated instead of inserted/removed.

In order to calculate effectively the difference between the two lists you have to use DiffUtil and it's up to the developer to build it as they have to provide the proper way of comparing the items in areItemsTheSame and areContentsTheSame. You can find a sample implementation from Google here.

Another flaw related to the code above that I noticed is that in the first comparison (previousState == state) if the state is different than LOADED, e.g. it changed from EMPTY to EMPTY, it will notify wrong positions in the adapter as it is using the current/previous content item count but in reality there is only one item content.

Let me know your thoughts.

[yccheok]

Thanks. I don't know there is such thing called DiffUtil

For EMPTY to EMPTY, isn't the code will be

if (1 == 1) {
        sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                section,
                0,
                1
        );
}

I can't see what's wrong with the code during EMPTY to EMPTY. I thought the above code will notify correct content item in EMPTY state?

[luizgrp]

You are right, sorry, it should be fine!

[yccheok]

Hi @luizgrp

How are you doing?

Today, I double check the code again and again. I think you are right regarding this in your previous thread

Another flaw related to the code above that I noticed is that in the first comparison (previousState == state) if the state is different than LOADED, e.g. it changed from EMPTY to EMPTY,

If current state is EMPTY, statement int contentItemsTotal = section.getContentItemsTotal(); is meaningless. It is not returning 1. getContentItemsTotal value will only be meaningful, when the state is in LOADED.

Is it be more accurate, if my code is being changed to

public static void notifyDataSetChangedInSection(
        SectionedRecyclerViewAdapter sectionedRecyclerViewAdapter,
        Section section,
        Section.State previousState,
        int previousContentItemsCount
) {
    final Section.State state = section.getState();

    if (previousState == state) {
        if (state == Section.State.LOADED) {
            final int contentItemsTotal = section.getContentItemsTotal();

            if (contentItemsTotal == previousContentItemsCount) {
                sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                        section,
                        0,
                        contentItemsTotal
                );
            } else if (contentItemsTotal > previousContentItemsCount) {
                sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                        section,
                        0,
                        previousContentItemsCount
                );

                final int count = contentItemsTotal - previousContentItemsCount;

                sectionedRecyclerViewAdapter.notifyItemRangeInsertedInSection(
                        section,
                        previousContentItemsCount,
                        count
                );
            } else if (contentItemsTotal < previousContentItemsCount) {
                sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                        section,
                        0,
                        contentItemsTotal
                );

                final int count = previousContentItemsCount - contentItemsTotal;

                sectionedRecyclerViewAdapter.notifyItemRangeRemovedFromSection(
                        section,
                        contentItemsTotal,
                        count
                );
            }
        } else {
            // Don't use section.getSectionItemsTotal(), as it takes account into header and
            // footer. Just hard code it to 1.
            sectionedRecyclerViewAdapter.notifyItemRangeChangedInSection(
                    section,
                    0,
                    1
            );
        }
    } else {
        if (state == Section.State.LOADED) {
            sectionedRecyclerViewAdapter.notifyStateChangedToLoaded(section, previousState);
        } else if (previousState == Section.State.LOADED) {
            sectionedRecyclerViewAdapter.notifyStateChangedFromLoaded(section, previousContentItemsCount);
        } else {
            sectionedRecyclerViewAdapter.notifyNotLoadedStateChanged(section, previousState);
        }
    }
}

[luizgrp]

nice catch! yes, it makes more sense now!