-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: function array_group_by #7438
Conversation
Please fix the PHPStan error:
|
I tried different assignments, incl. string only, but i still get the same error that the function always evaluates to false. But this function call is important for the code to work. Any suggestions? |
@rumpfc |
Please run: |
The references are absolutely necessary. Each iteration of I can try to implement a recursive alternative (similar how |
I checked the behavior. It seems you are correct, and the PHPStan error seems to be false positive. If you cannot rewrite easily or if the rewrite code will be more complicated, I suggest you suppress the PHPStan error. |
Can you add test case for dot array syntax? |
They are already included. See the final |
Oh, I missed that. Thank you. |
The recursive approach satisfies both rector and phpstan, but I feel like it requires more memory. TIme performance should be the same. |
My quick benchmark on macOS shows recursive is slower, but the memory usage are the same. memory usage: 114232 public function test()
{
$iterator = new \CodeIgniter\Debug\Iterator();
$memory_base = memory_get_usage();
$iterator->add('array_group_by', static function () {
$indexes = ['gender', 'hr.department'];
$data = [
[
'id' => 1,
'first_name' => 'Urbano',
'gender' => null,
'hr' => [
'country' => 'Canada',
'department' => 'Engineering',
],
],
[
'id' => 2,
'first_name' => 'Case',
'gender' => 'Male',
'hr' => [
'country' => null,
'department' => 'Marketing',
],
],
[
'id' => 3,
'first_name' => 'Emera',
'gender' => 'Female',
'hr' => [
'country' => 'France',
'department' => 'Engineering',
],
],
[
'id' => 4,
'first_name' => 'Richy',
'gender' => null,
'hr' => [
'country' => null,
'department' => 'Sales',
],
],
[
'id' => 5,
'first_name' => 'Mandy',
'gender' => null,
'hr' => [
'country' => 'France',
'department' => 'Sales',
],
],
[
'id' => 6,
'first_name' => 'Risa',
'gender' => 'Female',
'hr' => [
'country' => null,
'department' => 'Engineering',
],
],
[
'id' => 7,
'first_name' => 'Alfred',
'gender' => 'Male',
'hr' => [
'country' => 'France',
'department' => 'Engineering',
],
],
[
'id' => 8,
'first_name' => 'Tabby',
'gender' => 'Male',
'hr' => [
'country' => 'France',
'department' => 'Marketing',
],
],
[
'id' => 9,
'first_name' => 'Ario',
'gender' => 'Male',
'hr' => [
'country' => null,
'department' => 'Sales',
],
],
[
'id' => 10,
'first_name' => 'Somerset',
'gender' => 'Male',
'hr' => [
'country' => 'Germany',
'department' => 'Marketing',
],
],
];
$actual = array_group_by($data, $indexes, true);
});
$htmlTable = $iterator->run(3000);
echo memory_get_peak_usage() - $memory_base;
echo $htmlTable; exit;
} |
@datamweb It is meaningless unless both (reference and recursive) results are compared in the same environment. |
@kenjis thanks, Can you suggest a link or reference for a better understanding of this issue? |
@rumpfc Please add this to the changelog to make people aware of this feature. |
@datamweb What is this issue? If we are talking about benchmarks, it means that they are meaningless unless we compare the results of running them in the same environment. The faster the machine, the faster the results. What we are discussing now in this GitHub issue is whether reference or recursive code is better on The PHPStan error is a bug in PHPStan; PHPStan does not seem to understand that references change values. |
Thanks, but there is "Others" section. --- a/user_guide_src/source/changelogs/v4.4.0.rst
+++ b/user_guide_src/source/changelogs/v4.4.0.rst
@@ -87,6 +87,9 @@ Libraries
Helpers and Functions
=====================
+- **Array:** Added :php:func:`array_group_by()` helper function to group data
+ values together. Supports dot-notation syntax.
+
Others
======
@@ -103,7 +106,6 @@ Others
- **Request:** Added ``IncomingRequest::setValidLocales()`` method to set valid locales.
- **Table:** Added ``Table::setSyncRowsWithHeading()`` method to synchronize row columns with headings. See :ref:`table-sync-rows-with-headings` for details.
- **Error Handling:** Now you can use :ref:`custom-exception-handlers`.
-- **Array:** Added ``array_group_by()`` helper function to group data values together. Supports dot-notation syntax.
Message Changes
*************** |
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.
LGTM!
Description
A function to group data rows by values defined by indexes. The function supports the "dot syntax". Optionally it can include empty values (where
empty($value) === true
) and puts them under key''
.The depth of return result equals
count($indexes)
.Example:
Returns following result
Checklist: