Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Great work! #1

Open
MathieuPerrais opened this issue Sep 10, 2021 · 0 comments
Open

Great work! #1

MathieuPerrais opened this issue Sep 10, 2021 · 0 comments

Comments

@MathieuPerrais
Copy link

MathieuPerrais commented Sep 10, 2021

I recently discovered your library and it's great!!
I wanted to build something similar for a long time but you did it better than I ever could, I just wanted to share that!

I was a bit surprised by the use of Combine's publisher for loading only 1 page then "completes".
I'm not an expert in RFP at all but my first thoughts were to use the stream of events to load the following pages then "complete" on the last one. Something roughly like this:
(you call .loadNextPage() and the initial subscriber gets another event with a new batch of reviews, mostly useful for wiring it up to an infinite scrolling tableview)

public class FeedPageLoader {
    private let feedLoader = FeedLoader()
    
    let publisher = CurrentValueSubject<Feed?, FeedLoader.Error>(nil)
    
    var hasMorePages: Bool {
        return publisher.value?.nextPage != nil
    }
    
    private func publish(page: Page) {
        feedLoader.fetch(page: page) { result in
            switch result {
            case .success(let feed):
                self.publisher.send(feed)
                if feed.nextPage == nil {
                    self.publisher.send(completion: .finished)
                }
            case .failure(let error):
                self.publisher.send(completion: .failure(error))
            }
        }
    }
    
    func loadPage(_ page: Page) {
        publish(page: page)
    }
    
    func loadNextPage() {
        guard let nextPage = publisher.value?.nextPage else {
            return
        }
        publish(page: nextPage)
    }
}

Obviously, this doesn't match your existing "Loader" behaviour that returns values and forgets. You kind of have to hang on to that one in memory.

What do you think of this approach, did you consider it and dropped it?

One last thing, I also thought of extending this library to offer a command to get the App ID from a "title" search, with the following API:
https://itunes.apple.com/search?term=TEXTQUERYAPPNAME&media=software

I'd love to fork this library or even better Contribute to this repo if I end up adding new features for my own use.
Would you be open to getting PRs against this repository?

Thanks a lot for your time and congrats, I really enjoyed browsing your code!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant