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

Paging enhancements #10

Open
brgrz opened this issue Aug 13, 2019 · 3 comments
Open

Paging enhancements #10

brgrz opened this issue Aug 13, 2019 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@brgrz
Copy link

brgrz commented Aug 13, 2019

Would it be possible to introduce overloads for Paged() that instead of current page use "take" and "skip" (where take is essentialy page size and skip is the number of items to skip)?

I generally find paging built around take and skip more intuitive than currentPage/pageSize.

(optionally)
Also it'd be nice if the PagedResults generic class returned the parameters we sent in (the current page/page size or take/skip).

Both enhancements would allow for less boilerplate code to support paging.

@mikebeaton
Copy link
Member

Mighty has to make some choice about the inputs to Paged, and I chose the inputs that Massive used - for obvious reasons. I think it would be confusing to add another set of paging methods which also take two ints but with different meanings - and I won't change the existing methods because I still value compatibility with Massive and also because it is pretty trivial to convert between different possible inputs to Paged. But don't forget that you can add your own C# extension methods to Mighty (or anything else), to give you the input semantics which you would like to use.

As regards your second request, I did think of that and it is not a bad idea. If you want to propose a PR for including the existing inputs currentPage & pageSize in the return object, you'd be welcome.

And equally, again - and perhaps obviously - you could always instead write your own extension methods for paging, which (make the above changes you want to the inputs, then) wrap the Mighty paging call, then copy Mighty's return values into your own return object, and finally add in the two input values which you want to come back out.

@mikebeaton
Copy link
Member

I'll close this for now, I've personally found doing paging with the current API which Rob Conery came up with for Massive works perfectly well. (And like I say, with object extension methods it's not hard to modify as you wish.)

If anyone else finds this is a problem, feel free to post on this issue and I'll re-open, or to post a new one.

@mikebeaton
Copy link
Member

mikebeaton commented Oct 21, 2019

@brgrz I've decided that I agree with you that it's useful for integration testing to have CurrentPage and PageSize in the PagedResults object. I'm adding them in the next version, which should be out shortly.

It's already added on the wip branch.

FWIW I'll still stick by the claim that you can't make an API perfect for everybody - there's more than one way to specify take/skip vs currentPage/pageSize and I'll stick with what Massive did for that bit (with the recommendation above to roll your own conversion code if you prefer take/skip).

@mikebeaton mikebeaton reopened this Oct 21, 2019
@mikebeaton mikebeaton added the enhancement New feature or request label Feb 20, 2020
@mikebeaton mikebeaton self-assigned this Feb 20, 2020
@mikebeaton mikebeaton mentioned this issue May 25, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants