Skip to content

Conversation

@florina-muntenescu
Copy link
Collaborator

No description provided.

if (response.isSuccessful) {
val repos = response.body()?.items ?: emptyList()
return RepoSearchResult.Success(repos)
return RepoSearchResult.Success(repos, response.body()?.total ?: 0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it look like if you just return the response object directly from the GithubService?

I think removing the RepoSearchResult wrapper may make it easier for folks to see retrofit <-> paging connection all in one place, and copy the code out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is only used in PagingSource then, it should be ok. I just don't want to Retrofit Response to leak in Repository.

@florina-muntenescu florina-muntenescu force-pushed the fm/paging_separators branch 4 times, most recently from 98c2f41 to 8115111 Compare March 4, 2020 18:22
@florina-muntenescu florina-muntenescu force-pushed the fm/paging_separators branch 2 times, most recently from a5be06e to 8e1f374 Compare March 12, 2020 15:46
@florina-muntenescu florina-muntenescu deleted the fm/paging_header branch March 12, 2020 16:03
@imsyf
Copy link

imsyf commented Jul 7, 2020

Hi, @florina-muntenescu, sorry to bother you.

I don't know why this PR is closed but I'm trying to replicate what you did here, displaying the total number of items returned from GitHub Search API.

But the problem is that the totalResultCount gets passed to ViewModel only with its initial value, which is 0 because the load suspend function in GithubPagingSource hasn't received response from the remote API yet. And when the response is received, that totalResultCount variable doesn't get updated in ViewModel.

Would you be able to help me fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants