Paginators and Other Collection Decorators Should Implement Enumerable or a Sub-Interface of Enumerable #33179
Unanswered
kaan-atakan
asked this question in
General
Replies: 1 comment
-
Will anyone respond to this? I might end up solving my use case by moving pagination out of the repository and into the controller, but it's a fairly major refactor (actually bigger than any of the modifications to the framework, that I have suggested) |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Sometimes I want to type hint a method's parameter to know whether it is a
Collection
or a decorator for aCollection
like aLengthAwarePaginator
. Most of the time it's because I want to use thepluck
method, but there can be other circumstances. The way the code is currently structured I can either:iterable
orTraversible
, both of which are too broad and don't guarantee I can use collection methods, or:Enumerable
orCollection
, both of which are too narrow and exclude decorators for the collectionsLooking at some of the decorators I notice that they use the
__call
magic method to call the underlyingCollection
's methods. These would need to be rolled out to regular intermediary methods. So for example:There are also other alternatives that would solve my use cases:
A
Collectible
Interface, to be implemented byCollection
and it's decorators that defines a methodgetCollection
ortoCollection
that returns the underlying collection for decorators and$this
if it's a subclass ofCollection
. With this method I could type hint aniterable
parameter and check to see if the parameter isCollectible
and then depending on the result either calling thegetCollection
method or passing the parameter to thecollect
helper. I don't prefer this because I think it will lead to repetitive verbosity near business logic, even though it would be more or less trivial to implement and shouldn't break anyone else's code.Changing
getArrayableItems
method of the Collection base class, that is called from the constructor) to check for thePaginator
contract (and any other decorator) and return the paginator's items array instead of calling thetoArray
method during theArrayable
check which returns an array of variousPaginator
properties rather than just the data. This way I could call collect on aniterable
parameter knowing that I will always get aCollection
instance with the correct data. This is would be a decent alternative but for the fact that it needlessly creates a copy of the collection. It might also break some people's code, though I doubt it.Changing decorators'
toArray
methods to return an array of the data instead of selected properties. Again, this way I could safely callcollect
on the parameter to ensure that I have a collection. This is more likely to break people's code, so I think it's the worst option.I personally think that rolling out the
Enumerable
methods in theCollection
decorators is the best way becausecollect
helper which will needlessly create a copy of the collection (or underlying collection with the decorators) when called on aCollection
or a decorator.iterable
and callingcollect
will also work with this method, making it functionally the same as the other three but more flexible.Enumerable
with another name (Collectible
maybe) and implement that in the relevant classes instead.I would be glad to do the work myself and issue a pull request, but I don't want to bother if it isn't going to make it into the main branch. I would also like some guidance on how to proceed. Ideally i would like to see this in laravel 6.x (since that's what most of my projects use) but I will settle for 8.x
Beta Was this translation helpful? Give feedback.
All reactions