moltin / ios-sdk

Swift SDK for the Moltin eCommerce API
https://moltin.github.io/ios-sdk
MIT License
37 stars 18 forks source link

Pagination utilities #44

Open craigtweedy opened 5 years ago

craigtweedy commented 5 years ago

Status

moltin.product.all { (result) in
    guard case .success(let firstPage) = result else { return }

    print(firstPage)
    var currentPage = firstPage

    while currentPage.links?["next"] != nil {
        currentPage.next(withConfig: moltin.config) { (paginatedResult) in
            guard case .success(let nextPage) = paginatedResult else { return }
            currentPage = nextPage
            print(currentPage)
        }
    }
}

Notes

Ideally, I'd not want the configuration to be specified again for each function, however, the response class does not have any understanding of the request that made it, so that would have to change and I'm not sure if I want that right now.

craigtweedy commented 5 years ago

@gje4 Any comments on the way it works? Thoughts about how to improve it so it doesn't require the config?

gje4 commented 5 years ago

Spent some more time with this. First thought would be a move to setting config to a global level. Either requiring the app to be set once at the appDelegate or even in the pList. It does have some restrictions that we have talked about in the past.

The other thought not sure if its worth exploring/thoughts. Move the next functionality to the Moltin class Something like public func next(forType object: , completionHandler: @escaping moltin.ProductRequest.DefaultCollectionRequestHandler) -> moltin.MoltinRequest

Or even tell them to pass the link public func next(nextData link: String, completionHandler: @escaping moltin.ProductRequest.DefaultCollectionRequestHandler) -> moltin.MoltinRequest

Think that may not be very efficient though.

I guess in closing, I do not think it is too much to ask for config. I also think from a practical stand point needing that much data for the UI is going to be the edge case. So probably not worth to much time.

craigtweedy commented 5 years ago

I'm pretty against making this config global - would prefer to keep everything contained to the Moltin module. It's on the implementing developer to then decide how best to manage both the class and the configuration.

The functions really already take the URL from the links dictionary, config is largely needed in order to construct other parameters such as authentication headers.

gje4 commented 5 years ago

All good with me, think it adds value and should be pushed.