[RFC Discussion] array_change_keys()

8 years ago by Colin O'Dell — view source

unread

Hello everyone,

I'd like to introduce a new RFC for your consideration and discussion:
https://wiki.php.net/rfc/array_change_keys This would add a new function
named array_change_keys() which simplifies the process of re-keying an
array.

PHP currently has an array_change_key_case() method, but this only allows
keys to be changed to upper- or lower-case; no method exists to change the
keys to some custom user-defined value. Although it's absolutely possible
to accomplish this without a special function, the alternate approaches
have several drawbacks:

Slower execution time compared to the proposed implementation.
Multiple lines and/or function calls are required.
Harder to understand the code's purpose with a quick glance.
The result of a "foreach" approach cannot be passed into another
function without using an intermediate variable.

A working implementation has been included with the RFC (huge thanks to
Jeremy Mikola for the heavy lifting here!) I've also requested this patch
be added to 3v4l.org; I'll notify everyone if/when that happens.

I'd greatly appreciate if you could review this RFC and let me know your
thoughts. I'd be happy to answer any questions you might have.

Regards,

Colin O'Dell

8 years ago by Fleshgrinder — view source

unread

I'd like to introduce a new RFC for your consideration and discussion:
https://wiki.php.net/rfc/array_change_keys This would add a new function
named array_change_keys() which simplifies the process of re-keying an
array.

+1 from my side, the sad fact that most coding standards require mixing
of snake_case and camelCase makes such a functionality really useful in
userland.

Does this RFC really require a 2/3 majority? I mean, it does not change
the syntax or alters anything. It is just about a single new function?

https://wiki.php.net/rfc/voting#required_majority

--
Richard "Fleshgrinder" Fussenegger

8 years ago by Colin O'Dell — view source

unread

Does this RFC really require a 2/3 majority? I mean, it does not change
the syntax or alters anything. It is just about a single new function?

https://wiki.php.net/rfc/voting#required_majority

Good catch! You're right, this RFC only adds a single new function and
does not modify the syntax or anything else. I've updated the RFC's voting
section accordingly.

Colin

8 years ago by Thomas Nunninger — view source

unread

Hello,

shouldn't it be possible to return null as new key? That way you say:
Use the next free integer index.

Not sure if returning null is wanted (as it could hide errors in the
callback) or needed in some real world use cases. But it would be more
in sync with $a[] = ...

Regards

Thomas

Hello everyone,

I'd like to introduce a new RFC for your consideration and discussion:
https://wiki.php.net/rfc/array_change_keys This would add a new function
named array_change_keys() which simplifies the process of re-keying an
array.

PHP currently has an array_change_key_case() method, but this only allows
keys to be changed to upper- or lower-case; no method exists to change the
keys to some custom user-defined value. Although it's absolutely possible
to accomplish this without a special function, the alternate approaches
have several drawbacks:

Slower execution time compared to the proposed implementation.

Multiple lines and/or function calls are required.

Harder to understand the code's purpose with a quick glance.

The result of a "foreach" approach cannot be passed into another
function without using an intermediate variable.

A working implementation has been included with the RFC (huge thanks to
Jeremy Mikola for the heavy lifting here!) I've also requested this patch
be added to 3v4l.org; I'll notify everyone if/when that happens.

I'd greatly appreciate if you could review this RFC and let me know your
thoughts. I'd be happy to answer any questions you might have.

Regards,

Colin O'Dell

8 years ago by Colin O'Dell — view source

unread

shouldn't it be possible to return null as new key? That way you say:
Use the next free integer index.

Not sure if returning null is wanted (as it could hide errors in the
callback) or needed in some real world use cases. But it would be more
in sync with $a[] = ...

That's an interesting idea, but it would go against how other array
functionality works.

For example, if you ran this:

var_dump( array_combine([null, null], ['foo', 'bar']) );

You'd get the following output:

array(1) { [""]=> string(3) "bar" }

The null you intended to be a key becomes an empty string instead, and the
latter value of "bar" overrides the former one.

You'll see this same behavior if you did the following:

$a = [];
$a[null] = 'foo';
$a[null] = 'bar';

So that's one way nulls are handled. The other way is to raise a warning
and skip that element, which is what array_flip (and this proposal) does:

var_dump( array_flip(['foo' => 'null']) );

Output:

Warning: array_flip(): Can only flip STRING and INTEGER values! in
/in/7bBvO on line 3 array(0) { }

Implementing the feature you mentioned would introduce a third approach,
which is something I'd like to avoid. Perhaps a future RFC could implement
this feature, either for just this one function or others as well?

Colin

8 years ago by Andreas Heigl — view source

unread

Hi all.

Just one question for clarification:

The RFC states "The new array returned by this function will contain the
same values in the same order, but with potentially different keys."

But further down it states, that on multiple times the same key the last
one will "win". So in that special case it's not the same array
anymore. Or did I miss something?

Cheers

Andreas

Am 29.05.16 um 15:13 schrieb Colin O'Dell:

Hello everyone,

I'd like to introduce a new RFC for your consideration and discussion:
https://wiki.php.net/rfc/array_change_keys This would add a new function
named array_change_keys() which simplifies the process of re-keying an
array.

PHP currently has an array_change_key_case() method, but this only allows
keys to be changed to upper- or lower-case; no method exists to change the
keys to some custom user-defined value. Although it's absolutely possible
to accomplish this without a special function, the alternate approaches
have several drawbacks:

Slower execution time compared to the proposed implementation.

Multiple lines and/or function calls are required.

Harder to understand the code's purpose with a quick glance.

The result of a "foreach" approach cannot be passed into another
function without using an intermediate variable.

A working implementation has been included with the RFC (huge thanks to
Jeremy Mikola for the heavy lifting here!) I've also requested this patch
be added to 3v4l.org; I'll notify everyone if/when that happens.

I'd greatly appreciate if you could review this RFC and let me know your
thoughts. I'd be happy to answer any questions you might have.

Regards,

Colin O'Dell

--
,,,
(o o)
+---------------------------------------------------------ooO-(_)-Ooo-+
| Andreas Heigl |
| mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" |
| http://andreas.heigl.org http://hei.gl/wiFKy7 |
+---------------------------------------------------------------------+
| http://hei.gl/root-ca |
+---------------------------------------------------------------------+

8 years ago by Colin O'Dell — view source

unread

The RFC states "The new array returned by this function will contain the
same values in the same order, but with potentially different keys."

But further down it states, that on multiple times the same key the last
one will "win". So in that special case it's not the same array
anymore. Or did I miss something?

You're right, I didn't word that first part very well. I've revised the
RFC to mention the two "exceptions to the rule" earlier.

Essentially this function is going to mirror the behavior of array_flip()
whenever an invalid or duplicate key is encountered.

Thanks for catching that!

Colin

8 years ago by Marco Pivetta — view source

unread

A bit skeptic here:

could you also provide the code for the benchmarks? I'd gladly measure
them with an accurate tool
do we really need another array function that is basically an
array_combine(array_map($someFunc, array_keys($arr)), $arr)?
and... do we really want another function that accepts arrays and not
generic Traversable (and therefore also generators)?
what is the real-world benefit over just
array_combine(array_map($someFunc, array_keys($arr)), $arr), except for
the two additional function calls here?

Cheers,

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Hello everyone,

I'd like to introduce a new RFC for your consideration and discussion:
https://wiki.php.net/rfc/array_change_keys This would add a new function
named array_change_keys() which simplifies the process of re-keying an
array.

PHP currently has an array_change_key_case() method, but this only allows
keys to be changed to upper- or lower-case; no method exists to change the
keys to some custom user-defined value. Although it's absolutely possible
to accomplish this without a special function, the alternate approaches
have several drawbacks:

Slower execution time compared to the proposed implementation.

Multiple lines and/or function calls are required.

Harder to understand the code's purpose with a quick glance.

The result of a "foreach" approach cannot be passed into another
function without using an intermediate variable.

A working implementation has been included with the RFC (huge thanks to
Jeremy Mikola for the heavy lifting here!) I've also requested this patch
be added to 3v4l.org; I'll notify everyone if/when that happens.

I'd greatly appreciate if you could review this RFC and let me know your
thoughts. I'd be happy to answer any questions you might have.

Regards,

Colin O'Dell

8 years ago by Colin O'Dell — view source

unread

Marco,

could you also provide the code for the benchmarks? I'd gladly measure
them with an accurate tool

Yeah that would be great! Here's the benchmark I was using:
https://gist.github.com/colinodell/872c1f0c92351af687347c0c8be4f253

do we really need another array function that is basically an
array_combine(array_map($someFunc, array_keys($arr)), $arr)?

While array_combine() technically works, the callback does not have access
to both the key AND value. Anyone needing access to both must currently
resort to using a foreach loop (unless there's some other clever
combination of functions that I'm missing).

We already have array_change_key_case(). IMO that function is way too
specific and not applicable in 99% of situations where you need to re-key
an array. This is why I'm proposing a general purpose function custom
tailored to rekeying an array.

I'll touch on some other advantages further down this message.

and... do we really want another function that accepts arrays and not

generic Traversable (and therefore also generators)?

Do the other array functions support this? If not then I'd argue that
adding Traversable support to array functions should be part of a broader
RFC which adds this support to all applicable functions at once.

In the mean time, you could certainly use iterator_to_array with this
proposed function:

$b = array_change_keys(iterator_to_array($a), $callback);

Doing this with array_combine() would require an intermediate variable to
prevent a "Fatal error: Uncaught Exception: Cannot traverse an already
closed generator":

$b = iterator_to_array($a);
$a = array_combine(array_map($callback, array_keys($b)), $b);

Even if all array functions did support generators you'd still need this
intermediate variable to prevent the double traversal - array_change_keys()
would not have this problem.

what is the real-world benefit over just
array_combine(array_map($someFunc, array_keys($arr)), $arr), except for
the two additional function calls here?

There are several IMO:

Callback has access to both the original key AND the original value.
The array_combine() approach can only access one or the other.
Better performance (hoping you can confirm this).
Purpose of the array_*() function is immediately obvious.
No need for intermediate variable when working with iterators means
function composition is possible.

Thanks for the feedback!

Colin

8 years ago by come@opensides.be — view source

unread

Le lundi 30 mai 2016, 01:40:21 Colin O'Dell a écrit :

Marco,

could you also provide the code for the benchmarks? I'd gladly measure
them with an accurate tool

Yeah that would be great! Here's the benchmark I was using:
https://gist.github.com/colinodell/872c1f0c92351af687347c0c8be4f253

do we really need another array function that is basically an
array_combine(array_map($someFunc, array_keys($arr)), $arr)?

While array_combine() technically works, the callback does not have access
to both the key AND value. Anyone needing access to both must currently
resort to using a foreach loop (unless there's some other clever
combination of functions that I'm missing).

Yes there is:
array_combine(
array_map($someFunc, array_keys($arr), $arr),
$arr
)
This way $someFunc gets key as first argument and value as second. (you might use array_values($arr) instead of $arr but I’m not sure it’s worth it)

Côme

8 years ago by Colin O'Dell — view source

unread

Yes there is:
array_combine(
array_map($someFunc, array_keys($arr), $arr),
$arr
)
This way $someFunc gets key as first argument and value as second. (you
might use array_values($arr) instead of $arr but I’m not sure it’s worth it)

Good call! I have updated the RFC to include this example as it best
matches the behavior of the proposed function.

Regards,

Colin

8 years ago by Nikita Popov — view source

unread

Marco,

could you also provide the code for the benchmarks? I'd gladly
measure
them with an accurate tool

Yeah that would be great! Here's the benchmark I was using:
https://gist.github.com/colinodell/872c1f0c92351af687347c0c8be4f253

I can't tell you what exactly this is benchmarking (probably a mix of
various caches ranging from the Zend MM, via the kernel, to the CPU). The
only thing it's pretty certainly not benchmarking are different
implementations of changing keys :) You're basically just penalizing
whichever implementation comes first.

For a more representative benchmark, reduce the number of elements by a
factor of 10 or hundred. In which case foreach should win at least against
the array_map+array_combine implementation.

Regards,
Nikita

8 years ago by Marco Pivetta — view source

unread

I benchmarked this more carefully at
https://github.com/Ocramius/array_change_keys-benchmark - the results are
indeed not matching what is represented in the RFC.

Specifically, array_change_keys has performance that is comparable to
array_combine + array_map, and looping is still faster by at least 35%.

This is mostly due to the function calls, which is what I expected.

Feel free to patch the benchmarks, should they be buggy. Also feel free to
add more scenarios.

Cheers,

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On Mon, May 30, 2016 at 3:40 AM, Colin O'Dell colinodell@gmail.com
wrote:

Marco,

could you also provide the code for the benchmarks? I'd gladly
measure
them with an accurate tool

Yeah that would be great! Here's the benchmark I was using:
https://gist.github.com/colinodell/872c1f0c92351af687347c0c8be4f253

I can't tell you what exactly this is benchmarking (probably a mix of
various caches ranging from the Zend MM, via the kernel, to the CPU). The
only thing it's pretty certainly not benchmarking are different
implementations of changing keys :) You're basically just penalizing
whichever implementation comes first.

For a more representative benchmark, reduce the number of elements by a
factor of 10 or hundred. In which case foreach should win at least against
the array_map+array_combine implementation.

Regards,
Nikita

8 years ago by Marco Pivetta — view source

unread

> Marco,
>
>
>> 1. could you also provide the code for the benchmarks? I'd gladly
>> measure them with an accurate tool
>>
>
> Yeah that would be great! Here's the benchmark I was using:
> https://gist.github.com/colinodell/872c1f0c92351af687347c0c8be4f253
>

Ported to https://github.com/Ocramius/array_change_keys-benchmark, thanks!

> 2. do we really need another array function that is basically an
>> `array_combine(array_map($someFunc, array_keys($arr)), $arr)`?
>>
>
> While `array_combine()` technically works, the callback does not have access
> to both the key AND value. Anyone needing access to both must currently
> resort to using a foreach loop (unless there's some other clever
> combination of functions that I'm missing).
>
> We already have `array_change_key_case()`. IMO that function is way too
> specific and not applicable in 99% of situations where you need to re-key
> an array. This is why I'm proposing a general purpose function custom
> tailored to rekeying an array.
>
> I'll touch on some other advantages further down this message.
>

From what I can see from the benchmarks, a userland implementation of this
is not really very different from a core implementation of it, as the only
removed overhead has pretty much an `O(1)`, while overhead is introduced by
forcing the key change via a callable.

>
> 3. and... do we really want another function that accepts arrays and not
>> generic Traversable (and therefore also generators)?
>>
>
> Do the other array functions support this? If not then I'd argue that
> adding Traversable support to array functions should be part of a broader
> RFC which adds this support to all applicable functions at once.
>

No, but I really don't care about the other functions. If something doesn't
support a `Traversable` in today's jungle of Generators and asynchronous
processing.

> In the mean time, you could certainly use iterator_to_array with this
> proposed function:
>
> $b = array_change_keys(iterator_to_array($a), $callback);
>

This is not usable: Iterators may be infinite, you can't know. Also, this
operation may cause a lot of blocking I/O, while instead we want to operate
only on the first N elements of an iterator.

Doing this with `array_combine()` would require an intermediate variable to
> prevent a "Fatal error: Uncaught Exception: Cannot traverse an already
> closed generator":
>
> $b = iterator_to_array($a);
> $a = array_combine(array_map($callback, array_keys($b)), $b);
>

Yes, this is not workable, as I mention above. Unpacking an iterator into
an array defeats the advantages of having an iterator.

> Even if all array functions did support generators you'd still need this
> intermediate variable to prevent the double traversal - array_change_keys()
> would not have this problem.
>

Yeah, I know what you mean here, but I don't think that the solution is to
add more `array_*` stuff that is as legacy as the previous.

> 4. what is the real-world benefit over just
>> `array_combine(array_map($someFunc, array_keys($arr)), $arr)`, except for
>> the two additional function calls here?
>>
>
> There are several IMO:
>
> 1. Callback has access to both the original key AND the original value.
> The `array_combine()` approach can only access one or the other.
>

A loop has that too, and a userland looping implementation is still faster

> 2. Better performance (hoping you can confirm this).
>

Sadly not :-(

> 3. Purpose of the array_*() function is immediately obvious.
>

Disagree, as mentioned in few edge cases in other responses (something
about numerical keys, IIRC: can't find the actual reply)

> 4. No need for intermediate variable when working with iterators means
> function composition is possible.
>

Function composition can be applied also with a userland implementation. My
main issue with this is that it doesn't need to exist in *core*. Doesn't
mean that it can't exist in userland ;-)

Cheers,

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

8 years ago by Colin O'Dell — view source

unread

Ported to https://github.com/Ocramius/array_change_keys-benchmark, thanks!

I've updated the RFC's benchmarks based on your tool. They confirm that
array_change_keys is faster than array_combine but slower than foreach (in
most cases). Thanks for helping with this!

No, but I really don't care about the other functions. If something
doesn't support a Traversable in today's jungle of Generators and
asynchronous processing.

While generators and async processing are nice, I don't think that means we
have to avoid work that doesn't fall under that umbrella. Or Is there
perhaps something unique about this particular function that warrants
Traversable support whereas other functions don't?

Purpose of the array_*() function is immediately obvious.

Disagree, as mentioned in few edge cases in other responses (something
about numerical keys, IIRC: can't find the actual reply)

I believe you're referring to the questions about returning null, invalid,
or duplicate keys. As I mentioned in my responses to those questions,
array_change_keys() was expressly designed to behave EXACTLY like
existing functions (such as array_flip):

Returning null as a key is not allowed (array_flip disallows this).
If you return a duplicate key, only the last value to return it will be
kept (just like calling array_flip with duplicate values or running:
$arr['foo'] = 1; $arr['foo'] = 2;)

Somebody else asked whether they could "return null" to use an automatic
numeric key - I responded "no" because "$arr[null]" is not the same as
"$arr[]".

Again, this RFC does NOT introduce any new edge cases or quirks. I'd
therefore respectfully ask that you re-consider this.

Function composition can be applied also with a userland implementation.
My main issue with this is that it doesn't need to exist in core. Doesn't
mean that it can't exist in userland ;-)

One could argue that array_change_key_case(), array_column(), random_int(),
password_hash(), and others don't strictly need to exist in core either.
However, they were all included because someone (or some group) felt that
they provided enough value to PHP developers and/or that their usefulness
outweighed the hassle of a userland implementation. All I ask is that we
give this RFC the same consideration - and if you still feel the same I
fully accept that :)

I appreciate the feedback as always and have updated the RFC with several
of these discussion points.

Colin

8 years ago by Colin O'Dell — view source

unread

array_change_keys() has been added to 3v4l.org if anyone would like to try
it online. Simple example: https://3v4l.org/vehTo/rfc#tabs