Remove getPaginator() from PaginatorInterface

[Art4]

What is the need and the output of the method PaginatorInterface::getPaginator()? I couldn't find any use or tests for this method. But I have to declare it in my Paginator? Why should a Paginator return itself?

There might be a need for this method in IlluminatePaginatorAdapter, but not in the Interface.

[Art4]

@garygreen said:

This function get's the real underling 'paginator' class passed when constructing the adapter, in case you need to act upon it.

Suppose the following class

<?php

use League\Fractal\Pagination\PaginatorInterface;

class MyOwnPaginator implements PaginatorInterface
{
/*
    public function getCurrentPage();
    public function getLastPage();
    public function getTotal();
    public function getCount();
    public function getPerPage();
    public function getUrl($page);
*/

    public function getPaginator()
    {
        // ???
        // return $this;
    }
}

Without an underlying paginator what should getPaginator() return? I agree with you this is helpful for Illuminate\Pagination. But if you aren't using Illuminate\Pagination and you havn't an underlying paginator you have to define the method nevertheless.

I think the interface should only care about methods that are used in fractal and not in other libraries.

[garygreen]

Fair point. It's possibly a good idea to remove it from the interface, as it does depend on how you construct the adapter but it depends on if people see a value in calling/accessing the underlying paginator -- It was added because this is merely an 'adapter' class and wasn't really intended to be the 'real' paginator instance. It's kinda strange to have your paginator built directly off the fractal paginator adapter interface if I'm honest (I really don't think it was designed with that in mind). I would have thought you would structure it like this:

class MyOwnPaginator
{
    public function getTheCurrentPage();
    public function getTheLastPage();
}


class MyOwnPaginatorAdapter implements PaginatorInterface
{
    public function __construct(MyOwnPaginator $paginator)
    {
        $this->paginator = $paginator;
    }

    public function getCurrentPage()
    {
        return $this->paginator->getTheCurrentPage();
    }

    public function getLastPage()
    {
        return $this->paginator->getTheLastPage();
    }

    public function getPaginator()
    {
        return $this->paginator;
    }
}

$paginatorAdapter = new MyOwnPaginatorAdapter(new MyOwnPaginator);

[garygreen]

A few more things to consider;

1. Your paginator is highly coupled to fractal, if you remove fractal from your app and your implementing it's paginator interface? Whoops. That doesn't make much sense.
2. Your method names changes on your paginator, you can easily change the adapter to call the appropriate function as per your paginator instance. With the way your doing it now, that wouldn't be possible.

I think the whole confusion springs from how you're building directly off the adapter interace, which I would strongly recommend against for the above reasons.

[Art4]

I'm aware of loose coupling and in fact I have an underlying paginator. But I think it isn't the task of fractal to guarantee the building of a loosely coupled adapter. This is part of the application architecture and should be in responsibility of the developer.

[Art4]

Another example: I have implemented PaginatorInterface with an underlying paginator, but I don't need the getPaginator() method so far but because of the interface I had to write it. The Interface forces me to write a method I would never call and this is useless code.

[philsturgeon]

Not entirely sure why that was ever there. Paginator interface changed a few times so it was probably useful at one point. The tests still pass so whatever.

[Art4]

Thanks