The time has come for splat operator

[orlangur]

No description provided.

[adragus-inviqa]

/cc #1047

[orlangur]

Hi @vkublytskyi 3207c4e is wrong - all changes I did in this PR does not require additional array_values call.

See:
https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/ObjectManager/Factory/Dynamic/Developer.php#L89
https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/ObjectManager/Factory/Dynamic/Production.php#L65
https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/ObjectManager/Factory/Compiled.php#L88

Please remove unnecessary array_values with further code updates and thanks for accepting PR 👍 :

/cc @okorshenko

[adragus-inviqa]

@orlangur - Don't think our code removes string keys, does it? Haven't looked too much, but I didn't see such a conversion.

[orlangur]

@adragus-inviqa this is ObjectManagerFactory internals where arrays are numeric by design. Of course methods available in client code supports string keys.

[vkublytskyi]

@orlangur, you are right array_values is not necessary at many places where it was added. But as ... operator require numeric array as argument usage of array_values is an implementation based on (operator) interface. Relying on fact that array is already contains numeric keys is an implementation which is based on behavior which leads to code coupling and less stable.

[orlangur]

@vkublytskyi, well, to me it looks like besides this PR similar changes were applied everywhere, then noticed that some of them cause fatal error and thus array_values blindly added everywhere which solved a problem but made implementation less elegant.

I didn't know such semantics of the operator thus faced with fatal as well when prepared PR and running tests allowed to catch problems.

As to me there is no justification for sub-optimal implementation in core having such a great unit/integration tests coverage. So, I didn't even touch Zend filter and concrete tests intentionally as such change wouldn't bring significant value but affected by PR placed are participating in hundreds of tests.

Thus it would be better to have the most compact implementation for crucial places like AbstractFactory or Interceptor and keep array_values for everything else, less frequently used.

[orlangur]

arrays_values call makes it two times slower (credits to https://phpixie.com/blog/benchmarking-the-php-splat-operator.html):

<?php
function test($a, $b, $c)
{

}

$params = array(1,2,3);

$t = time();
for($i=0; $i<10000000; $i++)
        call_user_func_array('test', $params);
echo time()-$t."\n";


$t = time();
for($i=0; $i<10000000; $i++) 
        test(...array_values($params));
echo time()-$t."\n";

$t = time();
for($i=0; $i<10000000; $i++) 
        test(...$params);
echo time()-$t."\n";

user@hh:~/workspace$ php -f test-splat.php
103
54
27

[okorshenko]

@orlangur as far as I remember you suggested in one of the discussions alternative Reflection framework. Unfortunately I can't find your comment right now. Could you please share your suggestion one more time about alternative library for Reflection.

Thank you