Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Refactors into a Page Class #81

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

myxoh
Copy link
Contributor

@myxoh myxoh commented Jul 8, 2022

Again doing a few things at once that you likely would want to breakup just to spark the discussion:

Splits Paginator into two classes:

  • Paginator is more of a controller class, and validates params and presents the results to the consumer
  • Page is left as an internal class where the real computation happens, which exposes results like: page (with cursor), records (just the records), page_info (metadata), total

Some other changes:

Of course this is more a draft - no added specs etc.
But I think a few benefits of this approach is to help reduce the class size; help clean up the specs (which right now are all testing the full paginator class and hence tests that should be fast - like validators, still take a lot of setup time)

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.

1 participant