Pagerfanta icon indicating copy to clipboard operation
Pagerfanta copied to clipboard

add mapping adapter

Open yjv opened this issue 9 years ago • 11 comments

adds an adapter that allows you to define a callback to be applied to all items before theyre given to the paginator

yjv avatar Dec 28 '15 23:12 yjv

@yjv very useful, we using similar behavior in one of our adapters https://github.com/Koc/Sphinxy/blob/master/Pagerfanta/Adapter/SphinxyQbAdapter.php#L33 . But I have some proposals:

  1. IMHO better name it with FilteringAdapter or WrappingAdapter
  2. What about call it once for slice instead of iterating slice and call for each item? This has next benefits:
    2.1. we can access to keys of the slice array
    2.2. slice will not converted into array (it can be object)

Koc avatar Dec 29 '15 11:12 Koc

@Koc thanks for all the feedback!

concerning your first point im naming it after how php names that functoinality itself. along those lines one named filter adapter i would think would filter out items not jsut map their values. naming it WrappingAdapter doesnt seem to me to tell what makes it special over any other adapter that wraps another adapter and does things with it. an example of an adapter that i think would also fall under the name wrapping adapter would be the AdaptersAdapter I have put in the other PR recently. So i figured a name a little more specific to what this one is doing ie mapping an item to another was more appropriate.

for your second point, I agree and ive made the changes to match!

thanks :)

yjv avatar Dec 29 '15 16:12 yjv

@Koc @stof changes made.

Thanks!

yjv avatar Dec 30 '15 17:12 yjv

@Koc @stof is this good to be merged?

thanks

yjv avatar Jan 10 '16 01:01 yjv

:+1: from me, but I haven't rights to merge anything to this repo.

ping @richsage @pablodip

Koc avatar Jan 10 '16 08:01 Koc

@yjv I've just added a couple more comments - nothing major though :)

richsage avatar Jan 11 '16 11:01 richsage

How about mapping inside the adapter, and not outside, like in tests? If mapping is done inside, then it is a mapping adapter, but if it it done out side it is just a callback adapter.

pablodip avatar Jan 11 '16 13:01 pablodip

@pablodip yeah good point. For some reason i didn't notice the CallbackAdapter until you pointed it out.

I changed it to that when it was pointed out to me that it would force all slices to be arrays in the end so even if the inner adapter returned an iterator the mapping adapter would return an array since it would have to get each value and map it.

After seeing that there is already a CallbackAdapter i agree that yeah unless this one applies the callback to each element alone its not useful enough. I could make the mapping adapter always loop through and return an array no matter what the inner adapter returns. I could also make or pull in a mapping iterator in the case of a traversable returned and call array_map when an array is returned from the inner iterator.

What are your thoughts?

Thanks!

yjv avatar Jan 11 '16 15:01 yjv

@richsage made fixes.

thanks!

yjv avatar Jan 11 '16 16:01 yjv

The current CallbackAdapter has a different behaviour, since it doesn't delegate to another adapter.

I'd probably go for an adapter open to composability, so that you could do mapping or whatever you wish. Something like TransformingSliceAdapter, where you could transform the slice the way you want. On op of that a MappingAdapter could be created to ease mapping.

pablodip avatar Jan 12 '16 20:01 pablodip

The current CallbackAdapter has a different behaviour, since it doesn't delegate to another adapter

That's not hard to do though:

$innerAdapter = //...

$adapter = new CallbackAdapter(
    function () use ($innerAdapter) {
        return $innerAdapter->getNbResults();
    },
    function ($offset, $length) use ($innerAdapter) {
        $data = $innerAdapter->getSlice($offset, $length);

        // Transform data

        return $data;
    }
));

stof avatar Mar 23 '16 17:03 stof