fermoya / SwiftUIPager

Native Pager in SwiftUI
MIT License
1.29k stars 171 forks source link

Why does data have to conform to RandomAccessCollection? #174

Closed quidnu closed 3 years ago

quidnu commented 3 years ago

Why does data have to conform to RandomAccessCollection?

My data is dynamically generated and an infinite stream which makes a dictionary make more sense to use as the type for "data" but it doesn't conform to RandomAccessCollection.

Shouldn't the only method that the data need to implement be indexing? i.e. data[index] -> pageData. If there were no page number you would need to be able to find the next and previous index but since you need to keep the page number to be able to jump to any page directly it seems unnecessary to require a BiDirectionalCollection.

The only place I see a RandomAccessCollection method being used is in InfiniteScrollExample where it invokes the .last method (but you can just use the page number there I think).

Am I mistaken?

fermoya commented 3 years ago

Pager is designed to display an Array of elements. This is because the elements must follow some kind of order. The reason I use RandomAccessCollection is for convenience but this collection is then converted into an Array.

I don’t understand the issue. Can’t you just use the dictionary values (notice the order won’t be always the same)?

quidnu commented 3 years ago

Using values would end up just being equivalent to using an array, but I want the data to be in a cache and would also potentially like to use negative page numbers which doesn't require any more work with a dictionary.

My understanding is that the only thing that data needs to be able to do is myData[currentPageNumber] -> dataForCurrentPage, so while there is an order the order is imposed by the page numbers not the data structure itself: Whatever data is returned by giving 4 as a parameter is the data the follows whatever data is returned by giving 3 as a parameter and this works the same with an array or a dictionary.

For example you would get the same behaviour if you had myData : Array = ['red', 'green', 'blue'] or myData : Dictionary = {0 : 'red', 1 : 'green', 2 : 'blue} but in the latter case I can also do myData = {0 : 'red', 1 : 'green', 2 : 'blue, -1 : 'orange'} which is convenient for dynamically loading pages preceding the first one. I can also more easily store only a subset of pages: {20: 'red', 21: 'blue, 19: 'green'}. If I wanted to do the same with an array the array indices would no longer correspond to the page numbers and I would have to maintain a mapping between the indices in the array and the pages. So in the sequential subset case ['green' , 'red', 'blue'] might be my data and might correspond to pages 19,20,21 (with pages 0-18 evicted to save memory).

With a dictionary I could even handle non-sequential pages {1 : 'foo', 18: 'bar'} (I might want to do this if some pages are especially memory hungry or maybe I don't have much control over what elements my caching structure can delete or I am not accessing the LRU cached data sequentially, maybe by tapping pager pips/dots or something like your "manual" example). I couldn't handle non-sequential pages using just an array.

I don't see why something like this isn't the API for "data":

protocol Indexable {
   func subscript(position: Int) -> Element
}

var data : Indexable

Is there a place in the code where data needs to conform to more than just the above protocol?

(I'm new to Swift so maybe there is a better way to write that. I thought a protocol that just requires subscript might already exist but the only thing I found was "Collection" which seems to require an order [but Dictionary conforms to Collection?])``

fermoya commented 3 years ago

In the implementation I use firstIndex(where:) and firstIndex(of:) to determine the order of elements and what item is focused depending on the page index. Internally, Pager uses a ForEach which uses a RandomAccessCollection:

@available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.0, *)
public struct ForEach<Data, ID, Content> where Data : RandomAccessCollection, ID : Hashable {

    /// The collection of underlying identified data that SwiftUI uses to create
    /// views dynamically.
    public var data: Data

    /// A function you can use to create content on demand using the underlying
    /// data.
    public var content: (Data.Element) -> Content
}

It just won't work with Dictionary.

You can use negative indexes with the current implementation, that's no problem. If you mean you'd want to insert new elements at the start, you can do so and the same at the end (see the Example Project). You just have to have in mind that if you add elements at the start, you'll need to update the page with the appropriate value (if page is 0 and you insert 5 elements, then page must be updated to 5 in order to avoid a programmatic scroll).

Pager needs an order that never changes and as far as I know a Dictionary doesn't keep an order, it's a different type of data access. If you have a Dictionary you can just iterate the (key, value) pairs and sort them by key (which would be an Int). Whenever you add a new value (having in mind your Dictionary should be marked as State) then your view will be reloaded, resorting the (key, value) pairs and passing the new data to Pager to update accordingly.

Pager is intended to work with a collection of unique elements that are sorted. See the definitions of Array:

An ordered, random-access collection.

and Dictionary:

A collection whose elements are key-value pairs.

And notice how Array fits the description I gave above.

quidnu commented 3 years ago

I think I see your point of view now: it's more natural to view the data as an ordered array if you also have non-focused elements (partially) visible, as all of your examples do, because you are viewing a "slice" of the data array.

But if you have only one element visible at a time and you want frequently add/remove elements as you would if using a cache it's more natural to view the data as a dictionary with the pages as keys. If you view it this way you see that having to store the data itself in an ordered fashion is unnecessarily restrictive: The only thing the API actually requires from the API user's point of view is to take a page and returns data for that page.

In the current implementation I think the places where RandomAccessCollection is currently used can be replaced by using the page numbers instead of the data itself. So instead of dataDisplayed you would have pagesDisplayed and you could ForEach over those instead and then index into the data to retrieve the corresponding data.

ForEach(pageNumbersDisplayed) { pageNumber in
return data[pageNumber]
}

In the case where you use firstIndex I think you would also do something similar

indexOfFocused = pageNumbersDisplayed.firstIndex(of: self.page ) 

The only place where there might be a problem is if you had to take a subsequence of the data such as data[firstPage...lastPage] but I don't see anything like that in the code.

My main aim is to be able to use a cache for the data since my data is memory intensive and I have many pages (think TikTok) and by allowing a dictionary to be used you can get caching essentially free through NSCache or some more fancy caching dictionary. You can also insert pages for free at the "beginning" of the data "array" for free in the case the user scrolls backwards by just inserting a negative page number as the key. You are right that you can implement all this by wrapping the dictionary in a class with an auxiliary array and resetting the page number to maintain the accounting but it seems much more clunky.

Maybe I'm wrong, I'm still thinking this over...

fermoya commented 3 years ago

I do use data[firstPage...lastPage]. Not directly but if you see how dataDisplayed is calculated eventually a slice of data is returned.

You mention you’d like to Pager to manage indexes and not the elements itself. That’s very interesting, but I think that it’s possible with the current implementation, am I wrong here?

You can do:

Pager(pagerModel,
           data: data) { item in
               MyView(item: item)
            }

But you could also do:

Pager(pagerModel,
           data: data.indexes) { index in
               MyView(item: data[index])
            }

So I guess, in your case, you could do:

Pager(pagerModel,
           data: dictionary.keys) { key in
               MyView(item: dictionary[key])
            }

Wouldn’t that work? This way you don’t manage elements but keys or indexes. Whatever data element it translates to is then up to your implementation

quidnu commented 3 years ago

Thanks, that might work. I'll give it a try.