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.

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

ArrayHelper $direction error when anonymous function #17372

Closed
mariovials opened this issue Jun 14, 2019 · 3 comments
Closed

ArrayHelper $direction error when anonymous function #17372

mariovials opened this issue Jun 14, 2019 · 3 comments
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug

Comments

@mariovials
Copy link

What steps will reproduce the problem?

    \yii\helpers\ArrayHelper::multisort($items, function($item) {
      return ['attr1', 'attr2']);
    },
    [SORT_ASC, SORT_DESC]);

What is the expected result?

sorted array

What do you get instead?

Invalid Argument – yii\base\InvalidArgumentException
The length of $direction parameter must be the same as that of $keys.
@samdark samdark added status:ready for adoption Feel free to implement this issue. type:bug Bug labels Jun 14, 2019
@samdark
Copy link
Member

samdark commented Jun 14, 2019

Yes, that's valid.

alexkart added a commit to alexkart/yii2 that referenced this issue Jun 17, 2019
@alexkart
Copy link
Contributor

alexkart commented Jun 17, 2019

@samdark, this example from the documentation is incorrect https://www.yiiframework.com/doc/guide/2.0/en/helper-array

Second argument that specifies keys to sort by can be a string if it's a single key, an array in case of multiple keys or an anonymous function like the following one:

ArrayHelper::multisort($data, function($item) {
    return isset($item['age']) ? ['age', 'name'] : 'name';
});

You can't use this function because you need to know sorting keys (the result of this function) before this line https://github.com/yiisoft/yii2/blob/master/framework/helpers/BaseArrayHelper.php#L651
but currently, this function never evaluated before this line and just passed to \yii\helpers\BaseArrayHelper::getColumn which will not return the column of the sorting array but will return ['age', 'name'] and those values will be sorted which makes no sense and don't sort an array.

Even this example is wrong even if it doesn't generate an error:

        $data = [
            ['age' => 30, 'name' => 'Alexander'],
            ['age' => 19, 'name' => 'Barney'],
            ['age' => 30, 'name' => 'Brian'],
        ];
        ArrayHelper::multisort($data, function () {
            return ['age', 'name'];
        });

@alexkart
Copy link
Contributor

alexkart commented Jun 17, 2019

This can be fixed by adding this line:

public static function multisort(&$array, $key, $direction = SORT_ASC, $sortFlag = SORT_REGULAR)
    {
        $key = $key instanceof \Closure ? call_user_func($key) : $key;
        ...

But, there is a test case which expects that anonymous function will be passed to the \yii\helpers\BaseArrayHelper::getColumn and will return modified values of the array (not keys that should be used for sorting):

public function testMultisortClosure()
    {
        $changelog = [
            '- Enh #123: test1',
            '- Bug #125: test2',
            '- Bug #123: test2',
            '- Enh: test3',
            '- Bug: test4',
        ];
        $i = 0;
        ArrayHelper::multisort($changelog, function ($line) use (&$i) {
            if (preg_match('/^- (Enh|Bug)( #\d+)?: .+$/', $line, $m)) {
                $o = ['Bug' => 'C', 'Enh' => 'D'];
                return $o[$m[1]] . ' ' . (!empty($m[2]) ? $m[2] : 'AAAA' . $i++);
            }

            return 'B' . $i++;
        }, SORT_ASC, SORT_NATURAL);
        $this->assertEquals([
            '- Bug #123: test2',
            '- Bug #125: test2',
            '- Bug: test4',
            '- Enh #123: test1',
            '- Enh: test3',
        ], $changelog);
    }

This is a conflict of different behaviors that anonymous function should have.
If the source of truth is multisort function, the correct behavior is that one that used in the test and explained in the dock block of the function:

     /*
     * @param string|\Closure|array $key the key(s) to be sorted by. This refers to a key name of the sub-array
     * elements, a property name of the objects, or an anonymous function returning the values for comparison
     * purpose. The anonymous function signature should be: `function($item)`.
     * To sort by multiple keys, provide an array of keys here.
     */

It says that function should return values for comparison, not keys.
So, I think that incorrect behavior is described in the docs https://www.yiiframework.com/doc/guide/2.0/en/helper-array

Example in the docs should be something like this:

        $data = [
            ['age' => 30, 'name' => 'Alexander'],
            ['age' => 19, 'name' => 'Barney'],
            ['age' => 30, 'name' => 'Brian'],
        ];
        ArrayHelper::multisort($data, function ($item) {
            return isset($item['age']) ? $item['age'] : $item['name'];
        }, SORT_DESC);

But you can sort only by one field in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug
Projects
None yet
Development

No branches or pull requests

3 participants