search

MightyOrm / Mighty

A new, small, dynamic micro-ORM. Highly compatible with Massive, but with many essential new features.
BSD 3-Clause "New" or "Revised" License
101 stars 20 forks source link

Paging enhancements #10

Open brgrz opened 4 years ago

brgrz commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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).