Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Allow Iterator to be used with current, next, reset, key functions »

Allow Iterator to be used with current, next, reset, key functions

php.internals

David Lundgren

9 years ago
Greetings, As suggested by several reviewers of a PR[1] I recently submitted, I'd like to get feedback on letting custom Iterators be used in the current, next, reset, and key functions. If this is something to move forward with, I'll need some help with the RFC process. Recent experiences trying to use these functions with a custom Iterator, and a bug[2], led me to try and fix this. BC will occur when an object implementing Iterator is passed to one of the functions, as the interface methods will be called, instead of returning the objects properties. The old behavior should be maintained for classes that do not implement Iterator. [1] https://github.com/php/php-src/pull/2176 [2] https://bugs.php.net/bug.php?id=49369 Thanks, Dave
-- David Lundgren dlundgren@syberisle.net 808.639.0365 GPG: 0x26F54D7F

Levi Morrison

9 years ago
On Wed, Oct 26, 2016 at 7:42 AM, David Lundgren <dlundgren@syberisle.net> wrote:
> Greetings, > > As suggested by several reviewers of a PR[1] I recently submitted, I'd > like to get feedback on letting custom Iterators be used in the current, > next, reset, and key functions. If this is something to move forward > with, I'll need some help with the RFC process. > > Recent experiences trying to use these functions with a custom Iterator, > and a bug[2], led me to try and fix this. BC will occur when an object > implementing Iterator is passed to one of the functions, as the > interface methods will be called, instead of returning the objects > properties. The old behavior should be maintained for classes that do > not implement Iterator. > > [1] https://github.com/php/php-src/pull/2176 > [2] https://bugs.php.net/bug.php?id=49369 > > Thanks, > Dave > -- > David Lundgren > dlundgren@syberisle.net > 808.639.0365 > GPG: 0x26F54D7F > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
I can't seem to recall specifics anymore but I do believe some people would greatly prefer to remove the internal array pointer for iteration and deprecate these functions. Maybe someone else can remember more? Nikita and Bob, maybe?

Nikita Popov

9 years ago
On Fri, Oct 28, 2016 at 2:43 PM, Levi Morrison <levim@php.net> wrote:
> On Wed, Oct 26, 2016 at 7:42 AM, David Lundgren <dlundgren@syberisle.net> > wrote: > > Greetings, > > > > As suggested by several reviewers of a PR[1] I recently submitted, I'd > > like to get feedback on letting custom Iterators be used in the current, > > next, reset, and key functions. If this is something to move forward > > with, I'll need some help with the RFC process. > > > > Recent experiences trying to use these functions with a custom Iterator, > > and a bug[2], led me to try and fix this. BC will occur when an object > > implementing Iterator is passed to one of the functions, as the > > interface methods will be called, instead of returning the objects > > properties. The old behavior should be maintained for classes that do > > not implement Iterator. > > > > [1] https://github.com/php/php-src/pull/2176 > > [2] https://bugs.php.net/bug.php?id=49369 > > > > Thanks, > > Dave > > -- > > David Lundgren > > dlundgren@syberisle.net > > 808.639.0365 > > GPG: 0x26F54D7F > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > > I can't seem to recall specifics anymore but I do believe some people > would greatly prefer to remove the internal array pointer for > iteration and deprecate these functions. Maybe someone else can > remember more? Nikita and Bob, maybe? >
Yeah, I'm one of those people. In PHP 7 the most important user of the IAP, foreach, was switched to use a more robust mechanism. I believe that at this point, the concept of an internal array pointer has outlived its usefulness and we should be working toward removing any APIs that publicly expose it, including each and the next/current/key family of functions. I will propose to deprecate each() in particular for PHP 7.2, because it is both more problematic and more useless than the others, but I'd like to see the others go away in the long term as well. If people want fine-grained control over array iteration, they should use an external iterator (aka ArrayIterator), not an internal one. Nikita

Pierre Joye

9 years ago
hi Nikita, On Sun, Oct 30, 2016 at 3:06 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> On Fri, Oct 28, 2016 at 2:43 PM, Levi Morrison <levim@php.net> wrote: > >> On Wed, Oct 26, 2016 at 7:42 AM, David Lundgren <dlundgren@syberisle.net> >> wrote: >> > Greetings, >> > >> > As suggested by several reviewers of a PR[1] I recently submitted, I'd >> > like to get feedback on letting custom Iterators be used in the current, >> > next, reset, and key functions. If this is something to move forward >> > with, I'll need some help with the RFC process. >> > >> > Recent experiences trying to use these functions with a custom Iterator, >> > and a bug[2], led me to try and fix this. BC will occur when an object >> > implementing Iterator is passed to one of the functions, as the >> > interface methods will be called, instead of returning the objects >> > properties. The old behavior should be maintained for classes that do >> > not implement Iterator. >> > >> > [1] https://github.com/php/php-src/pull/2176 >> > [2] https://bugs.php.net/bug.php?id=49369 >> > >> > Thanks, >> > Dave >> > -- >> > David Lundgren >> > dlundgren@syberisle.net >> > 808.639.0365 >> > GPG: 0x26F54D7F >> > >> > -- >> > PHP Internals - PHP Runtime Development Mailing List >> > To unsubscribe, visit: http://www.php.net/unsub.php >> >> I can't seem to recall specifics anymore but I do believe some people >> would greatly prefer to remove the internal array pointer for >> iteration and deprecate these functions. Maybe someone else can >> remember more? Nikita and Bob, maybe? >> > > Yeah, I'm one of those people. In PHP 7 the most important user of the IAP, > foreach, was switched to use a more robust mechanism. I believe that at > this point, the concept of an internal array pointer has outlived its > usefulness and we should be working toward removing any APIs that publicly > expose it, including each and the next/current/key family of functions. I > will propose to deprecate each() in particular for PHP 7.2, because it is > both more problematic and more useless than the others, but I'd like to see > the others go away in the long term as well. If people want fine-grained > control over array iteration, they should use an external iterator (aka > ArrayIterator), not an internal one.
How do iterators compare to a for/while/custom loops using next/current/etc? I did not bench it lately but last time there was a significant impact when using Iterators. I also think it is a good thing to clean up the ways to work with arrays or iterable but we have to be very careful how we do it. I am not sure an actual deprecation is a good thing in 7.x without breaking any kind of tests out there. Indeed it will not break code but it is rather bad to break tests and it is a very annoying thing to do while trying to be good at using the latest PHP version. I am not sure if a documentation+UPGRADE guide and all possible communications about such deprecations would be enough. Maybe only until we reach the last version before 8.0 (which can use actual deprecations warnings, given we have more than 7.2 before 8.0). Cheers,
-- Pierre @pierrejoye | http://www.libgd.org

Levi Morrison

9 years ago
> I also think it is a good thing to clean up the ways to work with > arrays or iterable but we have to be very careful how we do it. I am > not sure an actual deprecation is a good thing in 7.x without breaking > any kind of tests out there. Indeed it will not break code but it is > rather bad to break tests and it is a very annoying thing to do while > trying to be good at using the latest PHP version.
I understand the concern here but getting out deprecation notices out early would be more important in my opinion. Additionally when we decided to make PHP 7 our next release we had already released 5.6. I pushed for a 5.7 to allow for deprecation notices as well as more time to develop features for 7.0 but that's not the route we took. I point this out to show that waiting for the last 7.x release is not prudent.

Rowan Collins

9 years ago
On 30/10/2016 04:31, Pierre Joye wrote:
> I also think it is a good thing to clean up the ways to work with > arrays or iterable but we have to be very careful how we do it. I am > not sure an actual deprecation is a good thing in 7.x without breaking > any kind of tests out there. Indeed it will not break code but it is > rather bad to break tests and it is a very annoying thing to do while > trying to be good at using the latest PHP version.
I don't follow this logic at all: isn't the whole point of deprecation notices to make people notice them? If people run tests with E_DEPRECATED active, it's presumably because they want those tests to tell them if they're using something deprecated; if people don't want the deprecation notices to break their tests, they can configure their test runner to ignore them. So who exactly is helped by not including those notices? Regards,
-- Rowan Collins [IMSoP]

Rasmus Schultz

9 years ago
With regards to deprecating array-functions that operate on the internal pointer - I'd like to just remind you of the fact that some of these functions, reset() and end() in particular, are often used not because someone cares about moving the internal pointer, but as a means of getting the first/last items of an array. If the internal pointers are removed, I'd like to suggest we improve the reset() and end() functions rather than removing them - there is an annoyance with these two functions presently: because they operate on the internal pointer, the force you to introduce often meaningless intermediary variables. That is, for example: $last = end(explode(",", $comma_separated_values)); Doesn't work. One is forced to introduce a variable: $parts = explode(",", $comma_separated_values); $last = end($parts); If internal pointers are removed, maybe the reset() and end() functions could be made to actually work consistently? Whether applied to an expression or variable. On Sat, Oct 29, 2016 at 10:06 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

Nikita Popov

9 years ago
On Sun, Oct 30, 2016 at 12:24 PM, Rasmus Schultz <rasmus@mindplay.dk> wrote:
> With regards to deprecating array-functions that operate on the > internal pointer - I'd like to just remind you of the fact that some > of these functions, reset() and end() in particular, are often used > not because someone cares about moving the internal pointer, but as a > means of getting the first/last items of an array. > > If the internal pointers are removed, I'd like to suggest we improve > the reset() and end() functions rather than removing them - there is > an annoyance with these two functions presently: because they operate > on the internal pointer, the force you to introduce often meaningless > intermediary variables. > > That is, for example: > > $last = end(explode(",", $comma_separated_values)); > > Doesn't work. One is forced to introduce a variable: > > $parts = explode(",", $comma_separated_values); > $last = end($parts); > > If internal pointers are removed, maybe the reset() and end() > functions could be made to actually work consistently? Whether applied > to an expression or variable. >
Yeah, I've been thinking about this as well. reset() and end() do seem to be the most commonly used IAP functions -- while nobody actually cares about their effect on the IAP. The thing is, if we remove all other functions that allow users to inspect the IAP, then we can simply change reset() and end() to return the first/last element. As they would no longer actually modify the array, they would no longer accept their argument by reference, so end(explode(...)) would also become legal (and additionally, they would no longer have to do a full copy of their argument, like they currently sometimes need to do). Nikita

Fleshgrinder

9 years ago
On 10/30/2016 12:45 PM, Nikita Popov wrote:
> On Sun, Oct 30, 2016 at 12:24 PM, Rasmus Schultz <rasmus@mindplay.dk> wrote: > >> With regards to deprecating array-functions that operate on the >> internal pointer - I'd like to just remind you of the fact that some >> of these functions, reset() and end() in particular, are often used >> not because someone cares about moving the internal pointer, but as a >> means of getting the first/last items of an array. >> >> If the internal pointers are removed, I'd like to suggest we improve >> the reset() and end() functions rather than removing them - there is >> an annoyance with these two functions presently: because they operate >> on the internal pointer, the force you to introduce often meaningless >> intermediary variables. >> >> That is, for example: >> >> $last = end(explode(",", $comma_separated_values)); >> >> Doesn't work. One is forced to introduce a variable: >> >> $parts = explode(",", $comma_separated_values); >> $last = end($parts); >> >> If internal pointers are removed, maybe the reset() and end() >> functions could be made to actually work consistently? Whether applied >> to an expression or variable. >> > > Yeah, I've been thinking about this as well. reset() and end() do seem to > be the most commonly used IAP functions -- while nobody actually cares > about their effect on the IAP. The thing is, if we remove all other > functions that allow users to inspect the IAP, then we can simply change > reset() and end() to return the first/last element. As they would no longer > actually modify the array, they would no longer accept their argument by > reference, so end(explode(...)) would also become legal (and additionally, > they would no longer have to do a full copy of their argument, like they > currently sometimes need to do). > > Nikita >
I think it makes much more sense to deprecate all of them and introduce new ones that work in the way we want them to for several reasons. `end()` and `reset()` would have to continue working as they do until the next major release since people who use them might rely on the way they work. This means in effect that nobody can make her PHP 7 code PHP 8 ready up front. Instead we could directly introduce `array_first()` and `array_last()` with the next feature release -- even if the others are not deprecated -- and everyone can update their code or at least use it in new code. Changing `end()` and `reset()` in PHP 7 is imho way to dangerous! Another often asked/proposed question is whether they should accept traversables. I think especially for these two functions it's not easy to simply accept any kind of traversable. Think of an unbuffered database result which is effectively a remote pointer. Retrieving the first and last would mean to iterate all the results. An additional interface should be required for traversables in order to work with first and last.
-- Richard "Fleshgrinder" Fussenegger

Rasmus Schultz

9 years ago
On second thought, I agree with that - changing reset() and end() doesn't make sense, because people know them and expect them to work in a certain way. Likely a lot of people would actually continue to use them with intermediary variables the way they do today. Better to introduce a new pair of functions, since this will make it clear when consumer code depends on the new behavior - if we update the existing functions, that means you have to read code knowing which version of these very common (and very old) functions you were expecting to call. One thing though, since we have to introduce new functions, I would not suggest these be array_first() and array_last(), but rather simply first() and last(), and make then work with anything iterable, not just arrays.
> Retrieving the first and last would mean to iterate all the results.
Well, retrieving the last would - retrieving the first would mean iterating only over the first result. For a lot of use-cases, and unbuffered database results in particular, this is precisely what you'd want.
> An additional interface should be required for traversables in order to work with first and last.
I think it would be great to have that as an option - for cases where you can and want to optimize retrieval of the last item, but I don't think it should be required? For example, in the case of an unbuffered database query, there is likely no optimization that can be made for last() in the first place, since the database client/server are likely using a protocol that doesn't even allow you to skip to the last result; in that case, requiring everyone to implement a new interface, which does nothing, isn't meaningful. On Sun, Oct 30, 2016 at 1:07 PM, Fleshgrinder <php@fleshgrinder.com> wrote:

Fleshgrinder

9 years ago
On 10/30/2016 1:31 PM, Rasmus Schultz wrote:
> On second thought, I agree with that - changing reset() and end() > doesn't make sense, because people know them and expect them to work > in a certain way. Likely a lot of people would actually continue to > use them with intermediary variables the way they do today. Better > to introduce a new pair of functions, since this will make it clear > when consumer code depends on the new behavior - if we update the > existing functions, that means you have to read code knowing which > version of these very common (and very old) functions you were > expecting to call. > > One thing though, since we have to introduce new functions, I would > not suggest these be array_first() and array_last(), but rather > simply first() and last(), and make then work with anything iterable, > not just arrays. >
`first()` and `last()` are extremely generic names and I am still hoping to see nikic's scalar objects extension to land in core before 8. In this case one would always call `$x->first()` and `$x->last()`. Continuing with the `array_` prefix makes perfect sense to me, even if they accept `\Traversable` instances as well. We also expect `str_` prefixed functions to accept stringable objects, dont' we. It's just a prefix for grouping and not necessarily a restrictions regarding the types the function accepts. On 10/30/2016 1:31 PM, Rasmus Schultz wrote:
>> Retrieving the first and last would mean to iterate all the >> results. > > Well, retrieving the last would - retrieving the first would mean > iterating only over the first result. For a lot of use-cases, and > unbuffered database results in particular, this is precisely what > you'd want. > >> An additional interface should be required for traversables in >> order to work with first and last. > > I think it would be great to have that as an option - for cases > where you can and want to optimize retrieval of the last item, but I > don't think it should be required? For example, in the case of an > unbuffered database query, there is likely no optimization that can > be made for last() in the first place, since the database > client/server are likely using a protocol that doesn't even allow you > to skip to the last result; in that case, requiring everyone to > implement a new interface, which does nothing, isn't meaningful. >
Not requiring the interface means that we violate the Liskow's substitution principle, something I see too often in core and successful PHP software out there. What if a traversable does not want the first and last to be retrieved in this manner? Throw an exception? This violates the principle since it is not expected to behave in this way. Another possibility would it be to return null but then the question is, was null returned because the first or last element is null or because I cannot retrieve the first and last. Making it explicit is much better.
-- Richard "Fleshgrinder" Fussenegger

Rasmus Schultz

9 years ago
I think you misunderstand. I'm only suggesting that implementing first/last explicitly as an optimization be optional. If you call first() or last() on a traversable that doesn't implement this interface, it will simply traverse the first item or traverse to the end. In other words, it'll always work, so it doesn't violate anything. As said, in a lot of cases, such as traversing a stream of database records, there *is* no optimization you can make for last() because that's not possible with the driver. So in that case, what you're proposing, is that all existing traversables, such as DB adapters, shouldn't work with these functions at all - everyone should have to go and implement this interface first, even for traversable record sets where no optimization can be made anyway and all you can do is actually fast-forward to the last record by traversing the whole result set anyhow. The only thing you'll get by forcing implementation of this interface, is no existing traversable will work with these functions. In addition, when everyone starts implementing this interface, those libraries will lose backwards compatibility, since that interface will not be available on older versions. All so you can force people to implement an interface that, in many cases, contains redundant code that simply traverses the entire result set to the last item and returns it, for no meaningful reason. On Sun, Oct 30, 2016 at 1:47 PM, Fleshgrinder <php@fleshgrinder.com> wrote:

Fleshgrinder

9 years ago
On 10/30/2016 1:59 PM, Rasmus Schultz wrote:
> I think you misunderstand. > > I'm only suggesting that implementing first/last explicitly as an > optimization be optional. > > If you call first() or last() on a traversable that doesn't implement > this interface, it will simply traverse the first item or traverse to > the end. > > In other words, it'll always work, so it doesn't violate anything. > > As said, in a lot of cases, such as traversing a stream of database > records, there *is* no optimization you can make for last() because > that's not possible with the driver. > > So in that case, what you're proposing, is that all existing > traversables, such as DB adapters, shouldn't work with these functions > at all - everyone should have to go and implement this interface > first, even for traversable record sets where no optimization can be > made anyway and all you can do is actually fast-forward to the last > record by traversing the whole result set anyhow. > > The only thing you'll get by forcing implementation of this interface, > is no existing traversable will work with these functions. > > In addition, when everyone starts implementing this interface, those > libraries will lose backwards compatibility, since that interface will > not be available on older versions. > > All so you can force people to implement an interface that, in many > cases, contains redundant code that simply traverses the entire result > set to the last item and returns it, for no meaningful reason. >
What I am concerned with is the opt-out case of this functionality. What if I do not want that somebody misuses an unbuffered result set in such a way: selecting 1,000 records just to throw 998 away. Your proposal is very appealing in many ways but the opt-out means that I now need to trigger a notice or throw an exception because there is no other way to disallow this functionality. Especially in the light of unbuffered result sets and generators which can be traversed only once. Another case is every unsorted form of collection, what is first, what is last? How to you determine first and last if the keys of a traversable are objects? Fail? There are too many open questions in this regard. It's easier with the primitive arrays because they have their limitations clearly defined (maps keep insert order). First and last only make sense for certain data structures but not for all. Implementing something that just works might work for 80 % but not for the rest and they have the problem now. I know that we have no means to express proper data structures but this will not make it better. Whether it makes it worse is another question that might help to decide.
-- Richard "Fleshgrinder" Fussenegger

Alice Wonder

9 years ago
On 10/30/2016 05:31 AM, Rasmus Schultz wrote:
> On second thought, I agree with that - changing reset() and end() > doesn't make sense, because people know them and expect them to work > in a certain way. Likely a lot of people would actually continue to > use them with intermediary variables the way they do today.
Today? In PHP 5.6.x I didn't have to use intermediate variables. Being required to use them was new with PHP 7 and I suspect you will find a lot of sites still using pre 7 because they haven't taken the time to make sure every app they run works in PHP 7. All my production servers still run 5.6.x for that reason, I'm only now starting to port the apps I can now that it appears the PECL modules I need have finally all been updated. The php apps I am fixing are currently littered with code like this: $SEVENFIX = explode('.', $rs[0]->cover); if(end($SEVENFIX) == "whatever") { // do stuff } before trying the app in my 7.1.0RCx test servers they worked just fine as if(end(explode('.', $rs[0]->cover)) == "whatever") { // do stuff } So I suppose some people might "continue" to use them with intermediary but a lot of code that hasn't been ported to 7 doesn't currently.

Nikita Popov

9 years ago
On Sun, Oct 30, 2016 at 1:48 PM, Alice Wonder <alice@librelamp.com> wrote:
> On 10/30/2016 05:31 AM, Rasmus Schultz wrote: > >> On second thought, I agree with that - changing reset() and end() >> doesn't make sense, because people know them and expect them to work >> in a certain way. Likely a lot of people would actually continue to >> use them with intermediary variables the way they do today. >> > > Today? > > In PHP 5.6.x I didn't have to use intermediate variables. >
You did. You simply suppressed the error in PHP 5. In PHP 7 the error classification changed, so you're no longer suppressing it. See: https://3v4l.org/2R8Vp Prior to the PHP 7 release I suggested marking a number of functions (array_pop/shift, reset, end) as prefer-ref, so they could be used without warning, see http://markmail.org/message/peefyi46iz2amdbq. Nothing really came of it at the time. Just mentioning it here because we can already avoid the warning right now -- we don't have to drop the IAP first. Both issues are independent. However, introducing array_first/last is still better than using reset/end etc, because these functions can easily trigger an unnecessary array copy, while array_first/last wouldn't have that problem. Nikita

Rowan Collins

9 years ago
On 30/10/2016 12:07, Fleshgrinder wrote:
> I think it makes much more sense to deprecate all of them and introduce > new ones that work in the way we want them to for several reasons. > `end()` and `reset()` would have to continue working as they do until > the next major release since people who use them might rely on the way > they work. This means in effect that nobody can make her PHP 7 code PHP > 8 ready up front. > > Instead we could directly introduce `array_first()` and `array_last()` > with the next feature release -- even if the others are not deprecated > -- and everyone can update their code or at least use it in new code.
I've always felt like these functions were "missing" from the language - there are a number of ways you *can* get the first or last item in an array, but all have side effects, because they're not designed for this purpose. It just so happens that - most of the time - reset() and end() have the smallest side effects. I can't remember if it was an earlier discussion here, or somewhere else, that pointed to getting the first and last *key* as an extra use case worth considering. It's currently possible (if awkward) with key(). I think it would make sense to, in the same version: - deprecate reset(), end(), each(), and key() - introduce array_first(), array_last(), array_first_key() and array_last_key() - document replacements for whatever other use cases we can find examples of Doing them all at once makes the messaging much clearer - this family of functions is going to be removed; and here are the new functions you can start using immediately instead. Regards,
-- Rowan Collins [IMSoP]

Christoph Becker

9 years ago
On 30.10.2016 at 18:19, Rowan Collins wrote:
> On 30/10/2016 12:07, Fleshgrinder wrote: > >> I think it makes much more sense to deprecate all of them and introduce >> new ones that work in the way we want them to for several reasons. >> `end()` and `reset()` would have to continue working as they do until >> the next major release since people who use them might rely on the way >> they work. This means in effect that nobody can make her PHP 7 code PHP >> 8 ready up front. >> >> Instead we could directly introduce `array_first()` and `array_last()` >> with the next feature release -- even if the others are not deprecated >> -- and everyone can update their code or at least use it in new code. > > I've always felt like these functions were "missing" from the language - > there are a number of ways you *can* get the first or last item in an > array, but all have side effects, because they're not designed for this > purpose. It just so happens that - most of the time - reset() and end() > have the smallest side effects. > > I can't remember if it was an earlier discussion here, or somewhere > else, that pointed to getting the first and last *key* as an extra use > case worth considering. It's currently possible (if awkward) with key(). > > I think it would make sense to, in the same version: > > - deprecate reset(), end(), each(), and key() > - introduce array_first(), array_last(), array_first_key() and > array_last_key() > - document replacements for whatever other use cases we can find > examples of > > Doing them all at once makes the messaging much clearer - this family of > functions is going to be removed; and here are the new functions you can > start using immediately instead.
I fully agree.
-- Christoph M. Becker

Alice Wonder

9 years ago
On 10/30/2016 10:19 AM, Rowan Collins wrote:
> > - deprecate reset(), end(), each(), and key() > - introduce array_first(), array_last(), array_first_key() and > array_last_key() > - document replacements for whatever other use cases we can find > examples of >
As a user I certainly like those names better. Semantic function names are a definite win in my view.

Larry Garfield

9 years ago
On 10/31/2016 11:26 AM, Alice Wonder wrote:
> On 10/30/2016 10:19 AM, Rowan Collins wrote: > >> >> - deprecate reset(), end(), each(), and key() >> - introduce array_first(), array_last(), array_first_key() and >> array_last_key() >> - document replacements for whatever other use cases we can find >> examples of >> > > As a user I certainly like those names better. Semantic function names > are a definite win in my view.
Question: Now that we have "iterable" as a type, should we be adding more array-specific functions or should such operations be designed, and named, to operate on any iterable? (first(), first_key(), etc.) --Larry Garfield

Levi Morrison

9 years ago
On Mon, Oct 31, 2016 at 11:27 AM, Larry Garfield <larry@garfieldtech.com> wrote:
> On 10/31/2016 11:26 AM, Alice Wonder wrote: >> >> On 10/30/2016 10:19 AM, Rowan Collins wrote: >> >>> >>> - deprecate reset(), end(), each(), and key() >>> - introduce array_first(), array_last(), array_first_key() and >>> array_last_key() >>> - document replacements for whatever other use cases we can find >>> examples of >>> >> >> As a user I certainly like those names better. Semantic function names are >> a definite win in my view. > > > > Question: Now that we have "iterable" as a type, should we be adding more > array-specific functions or should such operations be designed, and named, > to operate on any iterable? (first(), first_key(), etc.)
At least for last and last-key I do not think we should make them work with any iterable. My reasoning is that we do not want to consume the iterator for them because many iterators are not rewindable (such as generators). The first and first-key routines should generally work because `rewind(); valid(); current(); rewind();` doesn't contain a `next()` call. Rewinding without iterating works for generators: https://3v4l.org/O7ZKO Rewinding after a next() call: https://3v4l.org/QI434

Christoph Becker

9 years ago
On 31.10.2016 at 18:27m Larry Garfield wrote:
> On 10/31/2016 11:26 AM, Alice Wonder wrote: > >> On 10/30/2016 10:19 AM, Rowan Collins wrote: >> >>> - deprecate reset(), end(), each(), and key() >>> - introduce array_first(), array_last(), array_first_key() and >>> array_last_key() >>> - document replacements for whatever other use cases we can find >>> examples of >> >> As a user I certainly like those names better. Semantic function names >> are a definite win in my view. > > Question: Now that we have "iterable" as a type, should we be adding > more array-specific functions or should such operations be designed, and > named, to operate on any iterable? (first(), first_key(), etc.)
According to the earlier discussion it is not clear if we want these functions to operate on Iterables (consider Generators). Anyhow, we probably can't introduce a function named first() for BC reasons.
-- Christoph M. Becker

Yasuo Ohgaki

9 years ago
On Sun, Oct 30, 2016 at 8:24 PM, Rasmus Schultz <rasmus@mindplay.dk> wrote:
> I'd like to just remind you of the fact that some > of these functions, reset() and end() in particular, are often used > not because someone cares about moving the internal pointer, but as a > means of getting the first/last items of an array.
I'm one of them.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Levi Morrison

9 years ago
On Sun, Oct 30, 2016 at 7:30 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> On Sun, Oct 30, 2016 at 8:24 PM, Rasmus Schultz <rasmus@mindplay.dk> wrote: >> I'd like to just remind you of the fact that some >> of these functions, reset() and end() in particular, are often used >> not because someone cares about moving the internal pointer, but as a >> means of getting the first/last items of an array. > > I'm one of them. > > -- > Yasuo Ohgaki > yohgaki@ohgaki.net
Are you after the last value, the last key, or both?

Levi Morrison

9 years ago
On Sun, Oct 30, 2016 at 10:31 AM, Levi Morrison <levim@php.net> wrote:
> On Sun, Oct 30, 2016 at 7:30 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote: >> On Sun, Oct 30, 2016 at 8:24 PM, Rasmus Schultz <rasmus@mindplay.dk> wrote: >>> I'd like to just remind you of the fact that some >>> of these functions, reset() and end() in particular, are often used >>> not because someone cares about moving the internal pointer, but as a >>> means of getting the first/last items of an array. >> >> I'm one of them. >> >> -- >> Yasuo Ohgaki >> yohgaki@ohgaki.net > > Are you after the last value, the last key, or both?
To clarify, do you use the return value of these functions or rely on the IAP moving to also get the key?

Andrew Faulds

9 years ago
Hi Nikita, Nikita Popov wrote:
> On Fri, Oct 28, 2016 at 2:43 PM, Levi Morrison <levim@php.net> wrote: >> >> I can't seem to recall specifics anymore but I do believe some people >> would greatly prefer to remove the internal array pointer for >> iteration and deprecate these functions. Maybe someone else can >> remember more? Nikita and Bob, maybe? >> > > Yeah, I'm one of those people. In PHP 7 the most important user of the IAP, > foreach, was switched to use a more robust mechanism. I believe that at > this point, the concept of an internal array pointer has outlived its > usefulness and we should be working toward removing any APIs that publicly > expose it, including each and the next/current/key family of functions. I > will propose to deprecate each() in particular for PHP 7.2, because it is > both more problematic and more useless than the others, but I'd like to see > the others go away in the long term as well. If people want fine-grained > control over array iteration, they should use an external iterator (aka > ArrayIterator), not an internal one.
I'm sure I've suggested this before, but couldn't we drop the IAP without getting rid of these functions? They could maintain their own map of arrays to positions (effectively emulating the IAP), for example. Thanks.
-- Andrea Faulds https://ajf.me/

Andrew Faulds

9 years ago
Hi David, David Lundgren wrote:
> As suggested by several reviewers of a PR[1] I recently submitted, I'd > like to get feedback on letting custom Iterators be used in the current, > next, reset, and key functions. If this is something to move forward > with, I'll need some help with the RFC process. > > Recent experiences trying to use these functions with a custom Iterator, > and a bug[2], led me to try and fix this. BC will occur when an object > implementing Iterator is passed to one of the functions, as the > interface methods will be called, instead of returning the objects > properties. The old behavior should be maintained for classes that do > not implement Iterator.
A concern that strikes me is that it's possible some existing code uses these functions precisely because they don't support Iterators, because the author wants to iterate over an object's properties. What would be the proposed alternative in that case? Reflection? I think it's unfortunate that PHP has two iteration behaviours for objects, namely iterating over properties, and custom iterators. I think those ought to be separate, rather than the latter overriding the former. Thanks!
-- Andrea Faulds https://ajf.me/

David Lundgren

9 years ago
On 10/31/16 9:02 AM, Andrea Faulds wrote:
> > A concern that strikes me is that it's possible some existing code uses > these functions precisely because they don't support Iterators, because > the author wants to iterate over an object's properties. >
I hadn't thought about that particular use case. I also hadn't ever tried using these functions on non-arrays before I recently attempted using them with an Iterator.
> What would be the proposed alternative in that case? Reflection?
I would think that Reflection would be necessary to get the private/protected properties, but these functions currently allow iteration over private/protected properties of an object. This should be limited to public properties similar to the way foreach is limited. But that's another issue.
> I think it's unfortunate that PHP has two iteration behaviours for > objects, namely iterating over properties, and custom iterators. I think > those ought to be separate, rather than the latter overriding the former.
As a user it came as a surprise that I couldn't use an Iterator where an array was being used, as they are used interchangeable in some other contexts. I'll also admit that I don't normally read the docs until I have problems. The behavior with passing objects to those functions is only documented by a user comment. Dave
-- David Lundgren dlundgren@syberisle.net GPG: 0x26F54D7F

David Lundgren

9 years ago
Thank you everyone for your feedback. Nikita's post about wrapping the array with an ArrayIterator provides the best approach to allow an array or Traversable to be used. This will also provide the greatest backwards compatible usage for those of us writing user code that would need to be compatible with 5.6 and 7.x. I think it would be bad to remove these functions and force users to wrap arrays with ArrayIterator, just to manually iterate an array. After spending a little time looking through how these functions are being used in frameworks and libraries, removing them seems like it will cause more harm than good, even in a future version of the language. Toward Andrea's suggestion of replacing the IAP in these functions, I'll take a look and see what I can come up with. Considering the current usage of these functions, I don't think this will generate a lot of overhead. Again, thanks for the feedback! Dave On 10/26/16 8:42 AM, David Lundgren wrote:
> Greetings, > > As suggested by several reviewers of a PR[1] I recently submitted, I'd > like to get feedback on letting custom Iterators be used in the current, > next, reset, and key functions. If this is something to move forward > with, I'll need some help with the RFC process. > > Recent experiences trying to use these functions with a custom Iterator, > and a bug[2], led me to try and fix this. BC will occur when an object > implementing Iterator is passed to one of the functions, as the > interface methods will be called, instead of returning the objects > properties. The old behavior should be maintained for classes that do > not implement Iterator. > > [1] https://github.com/php/php-src/pull/2176 > [2] https://bugs.php.net/bug.php?id=49369 > > Thanks, > Dave >
-- David Lundgren dlundgren@syberisle.net GPG: 0x26F54D7F

Levi Morrison

9 years ago
> I think it would be bad to remove these functions and force users to > wrap arrays with ArrayIterator, just to manually iterate an array. After > spending a little time looking through how these functions are being > used in frameworks and libraries, removing them seems like it will cause > more harm than good, even in a future version of the language.
Can you show some examples of frameworks and libraries you found where it would cause more harm than good?

David Lundgren

9 years ago
On 11/3/16 2:58 PM, Levi Morrison wrote:
>> I think it would be bad to remove these functions and force users to >> wrap arrays with ArrayIterator, just to manually iterate an array. After >> spending a little time looking through how these functions are being >> used in frameworks and libraries, removing them seems like it will cause >> more harm than good, even in a future version of the language. > > Can you show some examples of frameworks and libraries you found where > it would cause more harm than good?
Apologies, that was a subjective statement. I also made it appear that the usage of these functions was greater than it may really be, and would cripple said projects, that was not my intention, and most definitely not the case. These are the ones I remember, and another way could be used if current/next/key were not available for these contexts: Yii CMarkdownParser: https://git.io/vXCiy ZF2 stdlib FastPriorityQueue: https://git.io/vXCiH Drupal DI Container: https://git.io/vXCPB Symfony MO file loader: https://git.io/vXCPK Symfony HttpFoundation Request: https://git.io/vXCXW Dave
-- David Lundgren dlundgren@syberisle.net GPG: 0x26F54D7F

Levi Morrison

9 years ago
On Thu, Nov 3, 2016 at 8:52 PM, David Lundgren <dlundgren@syberisle.net> wrote:
> On 11/3/16 2:58 PM, Levi Morrison wrote: >>> I think it would be bad to remove these functions and force users to >>> wrap arrays with ArrayIterator, just to manually iterate an array. After >>> spending a little time looking through how these functions are being >>> used in frameworks and libraries, removing them seems like it will cause >>> more harm than good, even in a future version of the language. >> >> Can you show some examples of frameworks and libraries you found where >> it would cause more harm than good? > > Apologies, that was a subjective statement. I also made it appear that > the usage of these functions was greater than it may really be, and > would cripple said projects, that was not my intention, and most > definitely not the case. > > These are the ones I remember, and another way could be used if > current/next/key were not available for these contexts: > > Yii CMarkdownParser: https://git.io/vXCiy > ZF2 stdlib FastPriorityQueue: https://git.io/vXCiH > Drupal DI Container: https://git.io/vXCPB > Symfony MO file loader: https://git.io/vXCPK > Symfony HttpFoundation Request: https://git.io/vXCXW > > > Dave > -- > David Lundgren > dlundgren@syberisle.net > GPG: 0x26F54D7F > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
Many of these just seem to use `current` to get the first item and would easily migrate to a new API such as `first()`.