Paginator Class - Refactor page function for better abstraction

[Robert Roskam]

So the existing Paginator class is great for the standard use case. 

But recently I wanted to be have a Paginator that would return at least per_page x items. I called it FlexPaginator. My use case was that I had a group_by value that I wanted to keep together on the same page, and so the rows per page needed to grow. When it was all said an done, I had a completely different looking page function on my FlexPaginator.

Here's the default one for reference:

def page(self, number):
    """
    Returns a Page object for the given 1-based page number.
    """
    number = self.validate_number(number)
    bottom = (number - 1) * self.per_page
    top = bottom + self.per_page
    if top + self.orphans >= self.count:
        top = self.count
    return self._get_page(self.object_list[bottom:top], number, self)

I'd like to refactor several things about it that would have made it more abstract and easier for me, but still keep it backwards compatible:

• rename bottom and top variables, instead self.first and self.last respectively
• turn their inline calculations into function calls to  a new, self.first() and self.last()
• move the entire self.object_list[bottom:top] into its own line
• move that specific logic into a function called self.get_page_objects
• store that function's value into  self.page_objects

Any comments?

[Tobias Kunze]

Hi Robert,

sounds good to me in general, just a few thoughts:

>I'd like to refactor several things about it that would have made it more
>abstract and easier for me, but still keep it backwards compatible:
>

> - rename bottom and top variables, instead self.first and self.last
> respectively
> - turn their inline calculations into function calls to a new,
> self.first() and self.last()

I don't really care if the variables are called top/bottom or
first/last, but I don't think naming the methods first() and last(), ORM
style, is a good idea, especially since those methods would return only
integers to slice `self.object_list` with. `_get_page_slice(number)`
(or similar) might be more readable.

> - move the entire self.object_list[bottom:top] into its own line
> - move that specific logic into a function called self.get_page_objects

I'm not really sure that *both* a method to get the page objects and a
method to calculate the start/end values are necessary (I fail to see a
use case), but I guess it won't hurt either.

> - store that function's value into self.page_objects

Could you expand on that? Since a superset of those objects is already
stored in `self.object_list`, it might be less redundant to just store
the slice object/the top/bottom values instead.

Best
Tobias