TextIterator changes

19 years ago by Andrei Zmievski — view source

unread

I am working on implementing BreakIterator API [1]. I considered two
approaches: making a separate class or merging the API into the
existing TextIterator. Having a separate class would be a bit cleaner,
but I can see people wanting to use it in foreach(), and since
TextIterator already provides a lot of BreakIterator's functionality, I
decided that merging would be the best option. However, there is an
overlap between the BreakIterator API and the current TextIterator one,
so there will have to be some changes.

TextIterator::current() signature will change from:

mixed current()

to:

mixed current(integer &$offset)

in order to support BreakIterator's functionality of returning offset
in current().

TextIterator::next() will return the offset of the next boundary
instead of returning nothing.
TextIterator::rewind() will be renamed to TextIterator::first() to
conform to BreakIterator's first()/last() API.

So this is heads up. Let me know if you have a problem with this.

-A

[1] http://icu.sourceforge.net/apiref/icu4c/ubrk_8h.html

19 years ago by Michael Wallner — view source

unread

Andrei Zmievski wrote:

I am working on implementing BreakIterator API [1]. I considered two
approaches: making a separate class or merging the API into the existing
TextIterator. Having a separate class would be a bit cleaner, but I can
see people wanting to use it in foreach(), and since TextIterator
already provides a lot of BreakIterator's functionality, I decided that
merging would be the best option. However, there is an overlap between
the BreakIterator API and the current TextIterator one, so there will
have to be some changes.

TextIterator::current() signature will change from:

mixed current()

to:

mixed current(integer &$offset)

This will raise the same issue as we have/had with the reflection API:
mike@honeybadger:~/build/php-unicode-debug$ cli -r 'interface i{function f();} class c implements i{function f($a){}}'

Fatal error: Declaration of c::f() must be compatible with that of i::f() in Command line code on line 1

Nah, don't look at me--I don't like that either.

TextIterator::rewind() will be renamed to TextIterator::first() to
conform to BreakIterator's first()/last() API.

So this is heads up. Let me know if you have a problem with this.

Huh? Rename or alias? It can't implement Iterator if there's no rewind() method.

Regards,

Michael

19 years ago by Andrei Zmievski — view source

unread

TextIterator does not implement Iterator interface, only Traversable.
It just happens to have functions of the same name as Iterator.

-Andrei

Andrei Zmievski wrote:

I am working on implementing BreakIterator API [1]. I considered two
approaches: making a separate class or merging the API into the
existing TextIterator. Having a separate class would be a bit
cleaner, but I can see people wanting to use it in foreach(), and
since TextIterator already provides a lot of BreakIterator's
functionality, I decided that merging would be the best option.
However, there is an overlap between the BreakIterator API and the
current TextIterator one, so there will have to be some changes.

TextIterator::current() signature will change from:
mixed current()
to:
mixed current(integer &$offset)

This will raise the same issue as we have/had with the reflection API:
mike@honeybadger:~/build/php-unicode-debug$ cli -r 'interface
i{function f();} class c implements i{function f($a){}}'

Fatal error: Declaration of c::f() must be compatible with that of
i::f() in Command line code on line 1

Nah, don't look at me--I don't like that either.

TextIterator::rewind() will be renamed to TextIterator::first() to
conform to BreakIterator's first()/last() API.
So this is heads up. Let me know if you have a problem with this.

Huh? Rename or alias? It can't implement Iterator if there's no
rewind() method.

Regards,

Michael

19 years ago by Michael Wallner — view source

unread

Andrei Zmievski wrote:

TextIterator does not implement Iterator interface, only Traversable. It
just happens to have functions of the same name as Iterator.

Ah, okay.
That leaves the picky OO strictness I personally don't like.
I think we should give PHP some freedom back in this area.

Regards,

Michael

19 years ago by Marcus Boerger — view source

unread

Hello Andrei,

you should reconsider giving it full iterator interface. Adding more
functionality is of course quite easy and no problem at all. Sharing an
interface also makes the whole thing feel much better as in no conflicts
as in consistent. Actually that's what interfaces are for.

Friday, June 23, 2006, 11:20:28 PM, you wrote:

TextIterator does not implement Iterator interface, only Traversable.
It just happens to have functions of the same name as Iterator.

-Andrei

Andrei Zmievski wrote:

I am working on implementing BreakIterator API [1]. I considered two
approaches: making a separate class or merging the API into the
existing TextIterator. Having a separate class would be a bit
cleaner, but I can see people wanting to use it in foreach(), and
since TextIterator already provides a lot of BreakIterator's
functionality, I decided that merging would be the best option.
However, there is an overlap between the BreakIterator API and the
current TextIterator one, so there will have to be some changes.

TextIterator::current() signature will change from:
mixed current()
to:
mixed current(integer &$offset)

This will raise the same issue as we have/had with the reflection API:
mike@honeybadger:~/build/php-unicode-debug$ cli -r 'interface
i{function f();} class c implements i{function f($a){}}'

Fatal error: Declaration of c::f() must be compatible with that of
i::f() in Command line code on line 1

Nah, don't look at me--I don't like that either.

TextIterator::rewind() will be renamed to TextIterator::first() to
conform to BreakIterator's first()/last() API.
So this is heads up. Let me know if you have a problem with this.

Huh? Rename or alias? It can't implement Iterator if there's no
rewind() method.

Best regards,
Marcus

19 years ago by Andrei Zmievski — view source

unread

Thanks, I know what they are for. I remember discussing Iterator vs.
Traversable with you on IRC, and for some reason we settled on
Traversable. Anyway, I'm changing it now.

-Andrei

Hello Andrei,

you should reconsider giving it full iterator interface. Adding more
functionality is of course quite easy and no problem at all.
Sharing an
interface also makes the whole thing feel much better as in no
conflicts
as in consistent. Actually that's what interfaces are for.

Friday, June 23, 2006, 11:20:28 PM, you wrote:

TextIterator does not implement Iterator interface, only Traversable.
It just happens to have functions of the same name as Iterator.

-Andrei

Andrei Zmievski wrote:

I am working on implementing BreakIterator API [1]. I considered
two
approaches: making a separate class or merging the API into the
existing TextIterator. Having a separate class would be a bit
cleaner, but I can see people wanting to use it in foreach(), and
since TextIterator already provides a lot of BreakIterator's
functionality, I decided that merging would be the best option.
However, there is an overlap between the BreakIterator API and the
current TextIterator one, so there will have to be some changes.

TextIterator::current() signature will change from:
mixed current()
to:
mixed current(integer &$offset)

This will raise the same issue as we have/had with the reflection
API:
mike@honeybadger:~/build/php-unicode-debug$ cli -r 'interface
i{function f();} class c implements i{function f($a){}}'

Fatal error: Declaration of c::f() must be compatible with that of
i::f() in Command line code on line 1

Nah, don't look at me--I don't like that either.

TextIterator::rewind() will be renamed to TextIterator::first
() to
conform to BreakIterator's first()/last() API.
So this is heads up. Let me know if you have a problem with this.

Huh? Rename or alias? It can't implement Iterator if there's no
rewind() method.

Best regards,
Marcus

19 years ago by Andrei Zmievski — view source

unread

Sean (on IRC) convinced me that something called *Iterator had better
implement Iterator interface (which TextIterator currently does not).
So changing method signatures is out of the question. Towards that, the
current functions will stay as they are, but I'll have to add
current_offset() (for getting the offset of the current element as
opposed to the element itself), and alias rewind() to first().

-Andrei

I am working on implementing BreakIterator API [1]. I considered two
approaches: making a separate class or merging the API into the
existing TextIterator. Having a separate class would be a bit cleaner,
but I can see people wanting to use it in foreach(), and since
TextIterator already provides a lot of BreakIterator's functionality,
I decided that merging would be the best option. However, there is an
overlap between the BreakIterator API and the current TextIterator
one, so there will have to be some changes.

TextIterator::current() signature will change from:

mixed current()

to:

mixed current(integer &$offset)

in order to support BreakIterator's functionality of returning offset
in current().

TextIterator::next() will return the offset of the next boundary
instead of returning nothing.

TextIterator::rewind() will be renamed to TextIterator::first() to
conform to BreakIterator's first()/last() API.

So this is heads up. Let me know if you have a problem with this.

-A

[1] http://icu.sourceforge.net/apiref/icu4c/ubrk_8h.html