Re: [RFC] [Discussion] Add new function `array_group`

> On 1.6.2023, at 00:59, Larry Garfield <larry@garfieldtech.com> wrote: > > On Wed, May 31, 2023, at 10:47 PM, Boro Sitnikovski wrote: > >> 1. The grouping that JavaScript/.NET/Lodash/Scala/etc. do (this should >> be the default of array_group) >> 2. The grouping that Haskell does, the one I proposed earlier (this can >> be altered in a flag within array_group) >> >> Based on this, I'd like to adjust my initial proposal, where we would >> have the following function: `function array_group(array $array, >> callable $callback, bool $consecutive_pairs = false): array {}` >> >> If the argument consecutive_pairs is false, it will use the >> >> function's return value to do the grouping ($callback accepting single >> element in this case) >> Otherwise, it will use the function's boolean return value to check if >> two consecutive elements need to be grouped ($callback accepting two >> elements in this case) >> >> (This approach seems to be consistent with array_filter in >> the sense >> the callback accepts one or two arguments) >> >> With a few example usages: >> >> ``` >> var_dump( array_group($arr1, function( $x ) { >> return (string) strlen( $x ); >> } ) ); >> // Producing ['3' => ['one', 'two'], '5' => >> ['three']] >> ``` >> >> Another one: >> >> ``` >> $arr = [-1,2,-3,-4,2,1,2,-3,1,1,2]; >> >> $groups = array_group( $arr, function( $p1, $p2 ) { >> return ($p1 > 0) == ($p2 > 0); >> } ); >> // Producing [[-1],[2],[-3,-4],[2,1,2],[-3],[1,1,2]] >> ``` > > This sounds like two separate functions in a trenchcoat. It should be two separate functions. > >> I believe this proposal captures many use cases, beyond the examples we >> discussed. Curious about any other thoughts. >> >> I'm also attaching a PoC patch that implements this. >> >> Attachments: >> * array_group.patch > > Side note: I don't think anyone reads patches sent to the list. I didn't even > realize it allowed attachments. :-) If you want someone to review code, a GitHub PR is the way to > go. > > You can also include benchmarks there. From experience, if you don't have a compelling > reason why this *needs* to be in C rather than PHP (which in this case boils down to performance > exclusively), you're not going to be able to convince people to add another random utility > function. That's just the reality these days. > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php Thank you for your thoughts. We can definitely separate them in two functions. Also thanks on the note for the patches - will keep it in mind :) Good point on the benchmarks, I will keep those in mind. Beyond performance, I think this has to do with developer experience pretty much, too. Almost all mainstream languages have a built-in grouping function, and in my honest opinion, it's time PHP has one too :) I think we can all agree grouping is a very common/basic functionality that we all meet with regularly.