# [PATCH] RecursiveTreeIterator implementation

- Thread: https://externals.io/message/38851
- Original archive: https://news-web.php.net/php.internals/38851
- Started by Arnaud Le Blanc on 2008-07-08T13:49:36+00:00
- Messages: 16
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #38851 — Arnaud Le Blanc — 2008-07-08T13:49:36+00:00

URL: https://externals.io/message/38851#38851

Hi,

I have seen a "NEEDS DEVELOPER" flag on the TODO for the "Implement 
RecursiveTreeIterator in C" task, so I would like to propose my 
implementation [1].

Currently it follows the PHP implementation, except that it does not have a 
$prefix public property (how about a private property and a setter method ?).

I hope this patch will be useful. Please say me if anything is wrong with it.

[1] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator.patch
[2] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator-phpt.tar.bz2

Regards,

Arnaud

---

## #38871 — Marcus Boerger — 2008-07-09T09:18:22+00:00

URL: https://externals.io/message/38851#38871 · In reply to: #38851

Hello Arnaud,

Tuesday, July 8, 2008, 3:49:36 PM, you wrote:

> Hi,

> I have seen a "NEEDS DEVELOPER" flag on the TODO for the "Implement 
> RecursiveTreeIterator in C" task, so I would like to propose my 
> implementation [1].

> Currently it follows the PHP implementation, except that it does not have a
> $prefix public property (how about a private property and a setter method ?).

> I hope this patch will be useful. Please say me if anything is wrong with it.

> [1] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator.patch
> [2] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator-phpt.tar.bz2

> Regards,

Looks very good! Many  thanks.

Best regards,
 Marcus

---

## #38876 — Arnaud Le Blanc — 2008-07-09T14:16:20+00:00

URL: https://externals.io/message/38851#38876 · In reply to: #38871

> 
> Looks very good! Many  thanks.
> 
> Best regards,
>  Marcus
> 
> 

Thanks :)

---

## #38880 — Marcus Boerger — 2008-07-09T18:06:14+00:00

URL: https://externals.io/message/38851#38880 · In reply to: #38876

Hello Arnaud,

  if you can provicde the same for HEAD then I'll submit it. And if you're
  fast enough we might even get it into 5.3.0 :-) Johannes, Lukas, how
  much time does he have?

  marcus

Wednesday, July 9, 2008, 4:16:20 PM, you wrote:

> 

> > […]

> Thanks :) 



Best regards,
 Marcus

---

## #38897 — Arnaud Le Blanc — 2008-07-10T12:16:33+00:00

URL: https://externals.io/message/38851#38897 · In reply to: #38880

Hello,

On Wednesday 09 July 2008 20:06:14 Marcus Boerger wrote:
> Hello Arnaud,
> 
>   if you can provicde the same for HEAD then I'll submit it. And if you're
>   fast enough we might even get it into 5.3.0 :-) Johannes, Lukas, how
>   much time does he have?
> 
>   marcus

Ok, I made it [1]. I never walked through the PHP6 code until now, I hope this 
is correct.

getPrefix and getPostfix return a binary or unicode string depending on 
UG(unicode).

getEntry returns the original value if it is binary or unicode. Else it 
converts it depending on UG(unicode), as the purpose of this method is to 
cast to string.

The convertion from array to string in getPrefix throws `E_NOTICE` errors, so I 
had to change the error_reporting setting in my test cases [2] for them to 
pass. I don't know if I may remove these errors or not.

current returns the type returned by getEntry.

key acts like getEntry.

I added a test (005) for the returned types. And an other (006) to test the 
case where RecursiveTreeIterator got an IteratorAggregate as first param. All 
tests should pass on 5.3 except 005.

[1] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator_HEAD.patch
[2] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator-phpt.tar.bz2

Arnaud

---

## #39010 — Arnaud Le Blanc — 2008-07-16T15:47:20+00:00

URL: https://externals.io/message/38851#39010 · In reply to: #38880

On Wednesday 09 July 2008 20:06:14 Marcus Boerger wrote:
> Hello Arnaud,
> 
>   if you can provicde the same for HEAD then I'll submit it. And if you're
>   fast enough we might even get it into 5.3.0 :-) Johannes, Lukas, how
>   much time does he have?
> 
>   marcus
> 

Hi,

I would like to add a method to allow to change the prefix strings, but are 
there any chances for this patch to be commited into 5.3 ?

Regards,

Arnaud

---

## #39017 — Lukas Kahwe Smith — 2008-07-16T19:41:37+00:00

URL: https://externals.io/message/38851#39017 · In reply to: #39010

> On Wednesday 09 July 2008 20:06:14 Marcus Boerger wrote:
> > […]
>
> Hi,
>
> I would like to add a method to allow to change the prefix strings,  
> but are
> there any chances for this patch to be commited into 5.3 ?


from the RM perspective .. if you get it commited before the 24th its  
ok with us .. the usefulness or relevance of this will be decided by  
Marcus/Etienne.

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

---

## #39038 — Marcus Boerger — 2008-07-17T07:24:50+00:00

URL: https://externals.io/message/38851#39038 · In reply to: #39010

Hello Arnaud,

  I will commit it afte I have reviewed and changed it a bit. Right now you
ignore conversoin errors. I'll change this to throw exceptions to be inline
with other code. However I am in Tel Aviv right now for a company work
meeting, so I have little time this week.

marcus

Wednesday, July 16, 2008, 5:47:20 PM, you wrote:

> On Wednesday 09 July 2008 20:06:14 Marcus Boerger wrote:
> > […]

> Hi,

> I would like to add a method to allow to change the prefix strings, but are
> there any chances for this patch to be commited into 5.3 ?

> Regards,

> Arnaud




Best regards,
 Marcus

---

## #39059 — Arnaud Le Blanc — 2008-07-17T16:55:50+00:00

URL: https://externals.io/message/38851#39059 · In reply to: #39038

Hello,

Ok, that's why I said that I didn't know what to do with these conversions. At 
now I just left the `E_NOTICE` errors.

Regards

> Hello Arnaud,
> 
>   I will commit it afte I have reviewed and changed it a bit. Right now you
> ignore conversoin errors. I'll change this to throw exceptions to be inline
> with other code. However I am in Tel Aviv right now for a company work
> meeting, so I have little time this week.
> 
> marcus
> 
> Wednesday, July 16, 2008, 5:47:20 PM, you wrote:
> 
> > […]
you're
> > […]
> 
> > […]
> 
> > […]
are
> > […]
> 
> > […]
> 
> > […]
> 
> 
> 
> 
> Best regards,
>  Marcus

---

## #39102 — Marcus Boerger — 2008-07-19T15:57:40+00:00

URL: https://externals.io/message/38851#39102 · In reply to: #38897

Hello Arnaud,

Thursday, July 10, 2008, 2:16:33 PM, you wrote:

> Hello,

> On Wednesday 09 July 2008 20:06:14 Marcus Boerger wrote:
> > […]

> Ok, I made it [1]. I never walked through the PHP6 code until now, I hope this
> is correct.

> getPrefix and getPostfix return a binary or unicode string depending on 
> UG(unicode).

> getEntry returns the original value if it is binary or unicode. Else it 
> converts it depending on UG(unicode), as the purpose of this method is to 
> cast to string.

> The convertion from array to string in getPrefix throws `E_NOTICE` errors, so I
> had to change the error_reporting setting in my test cases [2] for them to 
> pass. I don't know if I may remove these errors or not.

> current returns the type returned by getEntry.

> key acts like getEntry.

> I added a test (005) for the returned types. And an other (006) to test the
> case where RecursiveTreeIterator got an IteratorAggregate as first param. All
> tests should pass on 5.3 except 005.

> [1] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator_HEAD.patch
> [2] http://arnaud.lb.s3.amazonaws.com/RecursiveTreeIterator-phpt.tar.bz2

> Arnaud

Slightly modified and applied. Thanks for the good work.

Care to look into the MultipleIterator next?

marcus


Best regards,
 Marcus

---

## #39108 — Etienne Kneuss — 2008-07-20T02:17:08+00:00

URL: https://externals.io/message/38851#39108 · In reply to: #39102

Hello


--snip--
>
> Care to look into the MultipleIterator next?
>

If you do intend to do it, (or if you don't), please let us know so we
can prioritize things, and eventually focus on other things, before
the 24th.

Thanks in advance


-- 
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

---

## #39109 — Arnaud Le Blanc — 2008-07-20T03:20:31+00:00

URL: https://externals.io/message/38851#39109 · In reply to: #39102

Hello,

> Slightly modified and applied. Thanks for the good work.

Thanks :) 

> Care to look into the MultipleIterator next?

Ok, I will do that.

Etienne, I think I can do that before the 24th.

Regards,

Arnaud

---

## #39165 — Arnaud Le Blanc — 2008-07-22T09:23:47+00:00

URL: https://externals.io/message/38851#39165 · In reply to: #39109

Hello,

> > […]
> 

That's done, for 5_3 [1] and HEAD [2].
And a test [3][4] covering mostly all the cases.

[1] http://arnaud.lb.s3.amazonaws.com/MultipleIterator_5_3.patch
[2] http://arnaud.lb.s3.amazonaws.com/MultipleIterator_HEAD.patch
[3] http://arnaud.lb.s3.amazonaws.com/multiple_iterator_001_5_3.phpt
[4] http://arnaud.lb.s3.amazonaws.com/multiple_iterator_001_HEAD.phpt

Regards,

Arnaud

---

## #39207 — Marcus Boerger — 2008-07-22T23:16:14+00:00

URL: https://externals.io/message/38851#39207 · In reply to: #39165

Hello Arnaud,

Tuesday, July 22, 2008, 11:23:47 AM, you wrote:

> Hello,

> > […]

> That's done, for 5_3 [1] and HEAD [2].
> And a test [3][4] covering mostly all the cases.

> [1] http://arnaud.lb.s3.amazonaws.com/MultipleIterator_5_3.patch
> [2] http://arnaud.lb.s3.amazonaws.com/MultipleIterator_HEAD.patch
> [3] http://arnaud.lb.s3.amazonaws.com/multiple_iterator_001_5_3.phpt
> [4] http://arnaud.lb.s3.amazonaws.com/multiple_iterator_001_HEAD.phpt

Great work once more. I just moved the stuff all into spl_observer.c to
avoid increasing the amount of stuff that gets exported in the headers.
I also did a few minor tweaks and cleanups, nothing important though.

- setFlags parses directly into &intern->flags which is a long for that
  reason
- added !EG(exception) for the loops, though I have another patch that will
  make exception handling better

I've also upgraded you, you've got php-src access now, use it wisely :-)

Best regards,
 Marcus

---

## #39211 — Arnaud Le Blanc — 2008-07-23T06:36:53+00:00

URL: https://externals.io/message/38851#39211 · In reply to: #39207

Hello Marcus,

On Wednesday 23 July 2008 01:16:14 Marcus Boerger wrote:
> Hello Arnaud,
> 
> Tuesday, July 22, 2008, 11:23:47 AM, you wrote:
> 
> > […]
> 
> > […]
> 
> > […]
> 
> > […]
> 
> Great work once more. I just moved the stuff all into spl_observer.c to
> avoid increasing the amount of stuff that gets exported in the headers.
> I also did a few minor tweaks and cleanups, nothing important though.
> 
> - setFlags parses directly into &intern->flags which is a long for that
>   reason
> - added !EG(exception) for the loops, though I have another patch that will
>   make exception handling better
> 
> I've also upgraded you, you've got php-src access now, use it wisely :-)

Thanks :) I will try to not break everything ;) 

I did not found anything about "Spl(Array|Index|Member)Reference" on the TODO, 
are there any discussion on that ?

Regards,

Arnaud

---

## #39213 — Marcus Boerger — 2008-07-23T07:26:31+00:00

URL: https://externals.io/message/38851#39213 · In reply to: #39211

Hello Arnaud,

Wednesday, July 23, 2008, 8:36:53 AM, you wrote:

> Hello Marcus,

> On Wednesday 23 July 2008 01:16:14 Marcus Boerger wrote:
> > […]

> Thanks :) I will try to not break everything ;) 

> I did not found anything about "Spl(Array|Index|Member)Reference" on the TODO,
> are there any discussion on that ?

Nope. My idea was to have an overloaded object that proxies writes to an
Arraym an Index and so forth. When you write to such an instance than
instead the saved reference would be written to.

class SplArrayReference {
  function __construct(Array &$array, $index) {
    $this->array = &$array;
    $this->index = $index;
  }

  function __set($value) {
    $this->array[$this->index] = $value;
  }
  //......
}

Where __set is the c-level zend_object_set_t set handler. Given its
concept, structure and the fact that we have closures now, it would
probably better to do those in pecl/Spl_Types. But I might be wrong
in my assumtion that all of these can be done by closures anyway.

Best regards,
 Marcus