Some questions regarding onCreateViewHolder

[yccheok]

In onCreateViewHolder, I realize after

Section section = sections.get(entry.getKey());

There's isn't any need to check, whether section is visible or not. Does it mean, section is surely always visible?

The reason I have concern in onCreateViewHolder is that, currently, I have a Section. which whenever I set it to FAILED state, I will always hide it.

if (portfolioSection.getState() == Section.State.LOADING) {
    portfolioSection.setState(Section.State.FAILED);

    hideSection(sectionAdapter, portfolioSection);
}

private hideSection(SectionedRecyclerViewAdapter sectionedRecyclerViewAdapter, Section section) {
    if (false == section.isVisible()) {
        // Already invisible.
        return;
    }

    int sectionPosition = sectionedRecyclerViewAdapter.getSectionPosition(section);

    section.setVisible(false);

    sectionedRecyclerViewAdapter.notifySectionChangedToInvisible(section, sectionPosition);
}

Hence, I'm not exactly sure, whether I should need to provide a dummy view for FAILED state,

or it is OK not providing view for FAILED state, with assumption onCreateViewHolder will not be triggered for this invisible Section?

public class PortfolioSection extends Section {
    public PortfolioSection() {
        super(new SectionParameters.Builder(R.layout.trading_portfolio_item_section)
                .headerResourceId(R.layout.trading_portfolio_header_section)
                .loadingResourceId(R.layout.trading_portfolio_loading_section)
                .failedResourceId(R.layout.dummy_view)
                .build());

or

public class PortfolioSection extends Section {
    public PortfolioSection() {
        super(new SectionParameters.Builder(R.layout.trading_portfolio_item_section)
                .headerResourceId(R.layout.trading_portfolio_header_section)
                .loadingResourceId(R.layout.trading_portfolio_loading_section)
                .build());

I had tested the one without failed resource id. Works fine but I'm not sure whether it is correct, or I had missed out any edge case. Or, should I just provide an empty dummy view just to play safe?

Thanks.

[luizgrp]

Based on your code, it's relative safe to not provide the dummy resource, it would only be a problem if the view gets updated between your setState and setVisible call, which is unlikely.

However, I realised that before introducing the SectionParameters class, it was required by Section to provide resource ids for all the states in the constructor. Now it looks like it's optional, which is better but it was not intentional. In order to avoid developers changing a visible section to a state that doesn't have resource id we should check it when setState is called and throw an exception.

In your case you don't need to set the section state to failed, you could just set the visibility to false. But if you still want to change the state to failed for any particular reason, then I would recommend provide the empty resource, so you wouldn't be affected when we updated the library.

[yccheok]

Fair enough, I will then provide an dummy view for FAILED state for my case.

[luizgrp]

Added checks in c28e97f