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

Add missing methods in SlidingPaginationInterface #609

Open
wants to merge 1 commit into
base: master
from

Conversation

@sci3ma
Copy link

sci3ma commented Jan 24, 2020

No description provided.

@garak

This comment has been minimized.

Copy link
Collaborator

garak commented Jan 24, 2020

Why such methods are supposed to be "missing"?

@sci3ma

This comment has been minimized.

Copy link
Author

sci3ma commented Jan 24, 2020

Short answer:
You can't find them in the interface ;)

Long answer (based on my case):
I built some pageable object which serves more data (total pages, first, last, total items, next page, etc...) in rest api response. SlidingPagination provides most of these methods but:

  1. You can't mock SlidingPagination class because it's final
  2. Would be better to inject interface than base class

I wonder why not all methods are in this interface? Is there any purpose of that?

@garak

This comment has been minimized.

Copy link
Collaborator

garak commented Jan 24, 2020

Can't you use AbstractPagination or PaginationInterface instead?

@sci3ma

This comment has been minimized.

Copy link
Author

sci3ma commented Jan 24, 2020

No because AbstractPagination and PaginationInterface doesn't have such methods.
Like I mentioned only SlidingPagination has these methods but SlidingPaginationInterface has some lacks.

Maybe my point of view is too narrow and I don't fully understand your thoughts why SlidingPaginationInterface can't have such methods, then explain please.

@garak

This comment has been minimized.

Copy link
Collaborator

garak commented Jan 24, 2020

You can find relevant informations in issue #598 and related PR

@sci3ma

This comment has been minimized.

Copy link
Author

sci3ma commented Jan 24, 2020

Thanks, but it doesn't explain why we can't add methods in SlidingPaginationInterface which are exist in SlidingPagination. SlidingPaginationInterface in not fully consistent with SlidingPagination.

@garak

This comment has been minimized.

Copy link
Collaborator

garak commented Jan 24, 2020

I guess that interface was created just with needed (e.g. actually used) methods.

@sci3ma

This comment has been minimized.

Copy link
Author

sci3ma commented Jan 24, 2020

Yeah, perhaps. SlidingPagination evolved but someone forgot to update SlidingPaginationInterface and now it's time to do this.

@garak

This comment has been minimized.

Copy link
Collaborator

garak commented Jan 24, 2020

SlidingPaginationInterface had no evolution: it was created just a couple of weeks ago to solve aforementioned issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.