Paged list - selectExtension.getSelectedItems() loads _all_ the placeholders

[RobbWatershed]

About this issue

I'm using FastAdapter with PagedModelAdapter to display a large list of books.

Repro steps

1. Initialize a PagedList on a library with 70 books, using PagedList.Config.Builder and

• enablePlaceholders = true
• initialLoadSizeHint = 40
• pageSize = 20

2. View the first 10 items on a RecyclerView

3. Call selectExtension.getSelectedItems()

Expected behaviour
The selected items are returned; the DAO is never called
Only the first 40 items are loaded in the PagedList; the 30 others still are placeholders

Actual behaviour
As soon as selectExtension.getSelectedItems() is called, the DAO loads books 41 to 60 and 61 to 70.
All 70 items are loaded in the PagedList; no placeholder is left even though the RecyclerView has never gone beyond position 10.

Note
Loading all placeholders has both a performance impact and super weird side effects on my app.

If the current behaviour works as designed, could you at least add an option to avoid loading placeholders when collecting selected items ? Thanks a bunch !

Details

•  Used library version v4.1.2
•  Used support library version v1.0.0 (AndroidX)
•  Used gradle build tools version v3.5.3
•  Used tooling / Android Studio version AS v3.5.3
•  Other used libraries, potential conflicting libraries None that I know of

Checklist

• Searched for similar issues
• Checked out the sample application
• Read the README
• Checked out the CHANGELOG
• Checked out the MIGRATION GUIDE

[mikepenz]

@RobbWatershed the prefetching and everything is done by the PagedList from the paging support library.
that is actually nothing the FastAdapter does or decides.

The FastAdapter just acts and serves to have the adapter logic split apart from the item (UI) logic. And allows you to have own models different than the items.

In addition the FastAdapter provides selection, ... support

[RobbWatershed]

Let me be a little more precise. The interesting part of the call stack is as follows

allocatePlaceholders:619, PagedStorage (androidx.paging)
loadAroundInternal:170, TiledPagedList (androidx.paging)
loadAround:651, PagedList (androidx.paging)
getItem:239, AsyncPagedListDiffer (androidx.paging)
get:41, PagedItemListImpl (com.mikepenz.fastadapter.paged)
getAdapterItem:146, PagedModelAdapter (com.mikepenz.fastadapter.paged)
getRelativeInfo:551, FastAdapter (com.mikepenz.fastadapter)
recursive:818, FastAdapter (com.mikepenz.fastadapter)
recursive:803, FastAdapter (com.mikepenz.fastadapter)
getSelectedItems:86, SelectExtension (com.mikepenz.fastadapter.select)

In FastAdapter.recursive:816, there's a loop going through all the items.

Once that loop reaches the part of the list where placeholders are, FastAdapter.getRelativeInfo:551 calls getAdapterItem, which in turn eventually calls PagedList.loadAround and creates the described side-effect of loading all placeholders.

If I may suggest something, that optional flag I was imagining could be used in FastAdapter.getRelativeInfo:550 to avoid calling getAdapterItem when there's a placeholder.

The caller already checks if the returned item is null, so that might work.

What do you think about that ?

[mikepenz]

@RobbWatershed oh I see. that may actually be seen as a bug as it is not anticipated that the FastAdapter results in triggering to load in the whole list.

As you pointed out that's quite against the point of a PagedList :D

[mikepenz]

@RobbWatershed looking at it I may think that this may actually not be possible. as there are 2 problems:

1.) to get the selected items we have to iterate through the whole list to see what got selected
2.) we actually don't know if an element is resolved (or not) prior to asking getItem. And asking getItem itself already results in the trigger to load more data.

May I ask why you need to ask for getSelectedItems?

Perhaps there may be a better approach for your usecase whereas you keep a set of the identifiers of your selections, so you don't have to ask the whole list for what maybe got selected?

(side fact. in the past the FastAdapter kept a separate list of selections, but keeping an additional state served dangerous to keep it in sync with the main data set. and in 99% of the cases the length of the lists don't reach sizes where a single iteration through would be concerning)

[RobbWatershed]

> to get the selected items we have to iterate through the whole list to see what got selected
Parts of things I like in FastAdapter is the fact that there are two separate concepts :

• the Item which represents the visual element of the Android UI
• the Model which represents the unerlying "business" data

From my point of view, while iterating through the whole list is obviously necessary, iterating through all the Models is not. Why ? Because the user interacts with Items displayed on screen.
Thus, in my humble opinion, the concept of selection is tied to the Item and should be manipulated independently from the Model, it being loaded or not.

---

2.) we actually don't know if an element is resolved (or not) prior to asking getItem. And asking getItem itself already results in the trigger to load more data.

I have successfuly used PagedList.subList to "poke" the status of elements in my project. You can easily spot placeholders in the returned List : they have a value of null

Sample code

long nbPlaceholders = Stream.of(iLibrary.subList(minIndex, maxIndex)).filter(c -> c == null).count();

---

May I ask why you need to ask for getSelectedItems?

The use case I rely the most on getSelectedItems is as follows :

• The user selects multiple items on a list
• Action buttons appear on the top menu (share, delete, zip)
• When pushing one of the action buttons, I process all the selected items in one shot

Sample code

Set<ContentItem> selectedItems = selectExtension.getSelectedItems();
if (!selectedItems.isEmpty()) {
	List<Content> selectedContent = Stream.of(selectedItems).map(ContentItem::getContent).toList();
	askDeleteItems(selectedContent);
}

It's true that I could keep track of selected indexes on a separate list, but this is far from elegant. I'd rather discuss first with you to see if the path is blocked or not.

Thanks for your patience and time by the way ! 👍 👍

[mikepenz]

So I tried to bring in an item peeking possibility which hopefully resolves the problem for you. I will provide an alpha and it would be really amazing if you could give it a spin and report back

[mikepenz]

please have a look at v5.0.0-a02

[RobbWatershed]

Thanks for the quick action.

It looks like it doesn't work yet, although the code you commited looks good.

At first analysis, I'd say the problem is you simply forgot to override peekAdapterItem for PagedModelAdapter. It still uses the default one defined in IAdapter.

The object I'm using is an instance of PagedModelAdapter 😀

[mikepenz]

Currently uploading v5.0.0-a03 for you :D

[RobbWatershed]

Fantastic. My DAO is silent and the side effects are gone.

If I could hug you right now, I would definitely do it ! Thanks a lot for your time 👍 👍 👍

https://i.kym-cdn.com/photos/images/newsfeed/001/115/816/936.gif

[mikepenz]

I am glad I could help :D