Hi!
In the discussion on PR #2185[1] we've stumbled upon the issue of
internal classes implementing a count_elements handler (but not the
Countable interface).
Is it possible to detect this in userland? If not, I would suggest that
we introduce something like is_countable(), so users can check whether
count() may be safely called on a given value.
[1] https://github.com/php/php-src/pull/2185
--
Christoph M. Becker
2016-11-13 18:34 GMT+01:00 Christoph M. Becker cmbecker69@gmx.de:
Hi!
In the discussion on PR #2185[1] we've stumbled upon the issue of
internal classes implementing a count_elements handler (but not the
Countable interface).Is it possible to detect this in userland? If not, I would suggest that
we introduce something like is_countable(), so users can check whether
count()may be safely called on a given value.
How about just making those classes implement the interface instead?
Regards, Niklas
How about just making those classes implement the interface instead?
Christoph pointed out that there may be classes in extensions that use
count_elements.
Also it would make userland code simpler:
is_countable($thing)
vs
is_array($thing) || $thing implements \Countable
How about just making those classes implement the interface instead?
Christoph pointed out that there may be classes in extensions that use
count_elements.
Furthermore, letting those classes implement Countable could break BC.
Also it would make userland code simpler:
is_countable($thing)
vs
is_array($thing) || $thing implements \Countable
Good catch!
Note that Craig has added is_countable() to PR #2185 (which implements
the RFC). In my opinion, that is okay, but if anybody has objections,
we probably need another RFC.
--
Christoph M. Becker
On Mon, Nov 14, 2016 at 2:22 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:
How about just making those classes implement the interface instead?
Christoph pointed out that there may be classes in extensions that use
count_elements.Furthermore, letting those classes implement Countable could break BC.
How would this break BC?
SXE even already has a count() method, and count_elements respects
overrides of that method. Clearly SXE is supposed to implement Countable,
it was simply forgotten.
Also it would make userland code simpler:
is_countable($thing)
vs
is_array($thing) || $thing implements \CountableGood catch!
Note that Craig has added is_countable() to PR #2185 (which implements
the RFC). In my opinion, that is okay, but if anybody has objections,
we probably need another RFC.
This should be done as a separate change IMHO.
Nikita
Morning,
Just to chime in ... can you split the PR into the RFC, and the new thing
please.
Just another question on how we could make objects that have count_elements
(which is in object handlers) implement an interface on the class entry
(which is detached from handlers) ?
I'm sure it's doable, but it doesn't look very straight forward, some kind
of hacking in instanceof maybe ... not sure ...
Cheers
Joe
On Mon, Nov 14, 2016 at 2:22 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:How about just making those classes implement the interface instead?
Christoph pointed out that there may be classes in extensions that use
count_elements.Furthermore, letting those classes implement Countable could break BC.
How would this break BC?
SXE even already has a
count()method, and count_elements respects
overrides of that method. Clearly SXE is supposed to implement Countable,
it was simply forgotten.Also it would make userland code simpler:
is_countable($thing)
vs
is_array($thing) || $thing implements \CountableGood catch!
Note that Craig has added is_countable() to PR #2185 (which implements
the RFC). In my opinion, that is okay, but if anybody has objections,
we probably need another RFC.This should be done as a separate change IMHO.
Nikita
Just to chime in ... can you split the PR into the RFC, and the new thing
please.
Of course, that's done now: https://github.com/php/php-src/pull/2206
Morning,
Just to chime in ... can you split the PR into the RFC, and the new thing
please.Just another question on how we could make objects that have
count_elements (which is in object handlers) implement an interface on the
class entry (which is detached from handlers) ?
I'm sure it's doable, but it doesn't look very straight forward, some kind
of hacking in instanceof maybe ... not sure ...
I don't think this needs any magic. We simply implement the interface by
convention. Just like you are supposed to implement Traversable if you're
implementing get_iterator. Similarly you should implement Countable if
you're using count_elements.
Nikita
Afternoon,
A change in convention makes sense, and sounds much nicer than hacking
anything to make it work.
I do prefer this approach to yet-another-function.
Is changing convention all we would do though, or would we raise a warning
when handler is present and interface is not ?
Cheers
Joe
On Thu, Nov 17, 2016 at 8:30 AM, Joe Watkins pthreads@pthreads.org
wrote:Morning,
Just to chime in ... can you split the PR into the RFC, and the new thing
please.Just another question on how we could make objects that have
count_elements (which is in object handlers) implement an interface on the
class entry (which is detached from handlers) ?I'm sure it's doable, but it doesn't look very straight forward, some
kind of hacking in instanceof maybe ... not sure ...I don't think this needs any magic. We simply implement the interface by
convention. Just like you are supposed to implement Traversable if you're
implementing get_iterator. Similarly you should implement Countable if
you're using count_elements.Nikita
On Mon, Nov 14, 2016 at 2:22 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:How about just making those classes implement the interface instead?
Christoph pointed out that there may be classes in extensions that use
count_elements.Furthermore, letting those classes implement Countable could break BC.
How would this break BC?
SXE even already has a
count()method, and count_elements respects
overrides of that method. Clearly SXE is supposed to implement Countable,
it was simply forgotten.
Thinking about it, we most likely don't have to be concerned with some
theoretic BC break (such as !(instance of Countable)).
Thanks!
--
Christoph M. Becker