-
Notifications
You must be signed in to change notification settings - Fork 2
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
add mapping adapter #195
base: master
Are you sure you want to change the base?
add mapping adapter #195
Conversation
@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:
|
/** | ||
* MappingAdapter constructor. | ||
* @param AdapterInterface $innerAdapter | ||
* @param $callback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@param callable $callback
… used to pass it the entire slice instead of each item to allow for seeing keys and not forcing the slice into an array
@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 for your second point, I agree and ive made the changes to match! thanks :) |
$results = array(/* ... */); | ||
|
||
$adapter = new MappingAdapter(new ArrayAdapter($results), function ($item) { | ||
return $item + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs should be updated after your last changes
@@ -333,6 +333,26 @@ $results = array(/* ... */); | |||
$adapter = new FixedAdapter($nbResults, $results); | |||
``` | |||
|
|||
### MappingAdapter | |||
|
|||
This adapter allows you to define a callback t be applied to all items coming out of the adapter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo here: callback t
=> callback to
@yjv I've just added a couple more comments - nothing major though :) |
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 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! |
@richsage made fixes. thanks! |
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 |
That's not hard to do though:
|
adds an adapter that allows you to define a callback to be applied to all items before theyre given to the paginator