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
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Sometimes I want to type hint a method's parameter to know whether it is a
Collectionor a decorator for aCollectionlike aLengthAwarePaginator. Most of the time it's because I want to use thepluckmethod, but there can be other circumstances. The way the code is currently structured I can either:iterableorTraversible, both of which are too broad and don't guarantee I can use collection methods, or:EnumerableorCollection, both of which are too narrow and exclude decorators for the collectionsLooking at some of the decorators I notice that they use the
__callmagic 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
CollectibleInterface, to be implemented byCollectionand it's decorators that defines a methodgetCollectionortoCollectionthat returns the underlying collection for decorators and$thisif it's a subclass ofCollection. With this method I could type hint aniterableparameter and check to see if the parameter isCollectibleand then depending on the result either calling thegetCollectionmethod or passing the parameter to thecollecthelper. 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
getArrayableItemsmethod of the Collection base class, that is called from the constructor) to check for thePaginatorcontract (and any other decorator) and return the paginator's items array instead of calling thetoArraymethod during theArrayablecheck which returns an array of variousPaginatorproperties rather than just the data. This way I could call collect on aniterableparameter knowing that I will always get aCollectioninstance 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'
toArraymethods to return an array of the data instead of selected properties. Again, this way I could safely callcollecton 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
Enumerablemethods in theCollectiondecorators is the best way becausecollecthelper which will needlessly create a copy of the collection (or underlying collection with the decorators) when called on aCollectionor a decorator.iterableand callingcollectwill also work with this method, making it functionally the same as the other three but more flexible.Enumerablewith another name (Collectiblemaybe) 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