Skip to content
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.

Sign up for GitHub

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

Fix #20268: Minor optimisation in \yii\helpers\BaseArrayHelper::map #20269

Merged
merged 6 commits into from
Oct 27, 2024
Merged

Fix #20268: Minor optimisation in \yii\helpers\BaseArrayHelper::map #20269

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd9a6c4
Optimise ArrayHelper::map
chriscpty Oct 17, 2024
ca4bae9
Optimise ArrayHelper::map
chriscpty Oct 17, 2024
facef5a
improve ArrayHelper::map tests
chriscpty Oct 17, 2024
1fcadcd
fix ArrayHelper::map() for path strings
chriscpty Oct 18, 2024
7538551
fix ArrayHelper::map() for path strings
chriscpty Oct 18, 2024
4ed589f
use strpos rather than polyfill
chriscpty Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions framework/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Yii Framework 2 Change Log
- Enh #20247: Support for variadic console controller action methods (brandonkelly)
- Bug #20256: Add support for dropping views in MSSQL server when running migrate/fresh (ambrozt)
- Enh #20248: Add support for attaching behaviors in configurations with Closure (timkelty)
- Enh #20268: Minor optimisation in `\yii\helpers\BaseArrayHelper::map` (chriscpty)

2.0.51 July 18, 2024
--------------------
Expand Down
3 changes: 3 additions & 0 deletions framework/helpers/BaseArrayHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,9 @@ public static function getColumn($array, $name, $keepKeys = true)
*/
public static function map($array, $from, $to, $group = null)
{
if (is_string($from) && is_string($to) && $group === null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a check that both $from and $to don't contain a dot (.).
The ArrayHelper::getValue() function supports getting a value by a "path" string.
Without this check the following code would break:

$array = [
    [
        'prop' => [
            'a' => 'foo',
            'b' => 'bar',
        ],
    ],
];

ArrayHelper::map($array, 'prop.a', 'prop.b');

Copy link
Member

@samdark samdark Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It is necessary. @chriscpty would you please redo performance test with such a check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

performance with the added check:

before: 1729233314.6667
after: 1729233315.4382
elapsed: 0.77146291732788
before: 1729233315.4382
after: 1729233316.245
elapsed: 0.80677795410156
before: 1729233316.245
after: 1729233317.0443
elapsed: 0.79932498931885
before: 1729233317.0443
after: 1729233317.8304
elapsed: 0.78613996505737
before: 1729233317.8305
after: 1729233318.6256
elapsed: 0.79515385627747
bench size: 18000

I'll add the fix (and related tests) in a second

return array_column($array, $to, $from);
}
$result = [];
foreach ($array as $element) {
$key = static::getValue($element, $from);
Expand Down
40 changes: 39 additions & 1 deletion tests/framework/helpers/ArrayHelperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,44 @@ public function testMap()
'345' => 'ccc',
],
], $result);

$result = ArrayHelper::map($array,
static function (array $group) {
return $group['id'] . $group['name'];
},
static function (array $group) {
return $group['name'] . $group['class'];
}
);

$this->assertEquals([
'123aaa' => 'aaax',
'124bbb' => 'bbbx',
'345ccc' => 'cccy',
], $result);

$result = ArrayHelper::map($array,
static function (array $group) {
return $group['id'] . $group['name'];
},
static function (array $group) {
return $group['name'] . $group['class'];
},
static function (array $group) {
return $group['class'] . '-' . $group['class'];
}
);

$this->assertEquals([
'x-x' => [
'123aaa' => 'aaax',
'124bbb' => 'bbbx',
],
'y-y' => [
'345ccc' => 'cccy',
],
], $result);

}

public function testKeyExists()
Expand All @@ -759,7 +797,7 @@ public function testKeyExistsWithFloat()
if (version_compare(PHP_VERSION, '8.1.0', '>=')) {
$this->markTestSkipped('Using floats as array key is deprecated.');
}

$array = [
1 => 3,
2.2 => 4, // Note: Floats are cast to ints, which means that the fractional part will be truncated.
Expand Down
Loading