Hi everybody
I'd like to propose the introduction of warning when counting objects that
can't be counted.
The default behaviour is to return 1 for these objects, which can be
misleading and hide bugs when attempting to count iterable objects (eg
Generators). Adding a warning would alert developers to the issue
https://wiki.php.net/rfc/counting_non_countables
Thanks,
Craig
Hi everybody
I'd like to propose the introduction of warning when counting objects that
can't be counted.The default behaviour is to return 1 for these objects, which can be
misleading and hide bugs when attempting to count iterable objects (eg
Generators). Adding a warning would alert developers to the issuehttps://wiki.php.net/rfc/counting_non_countables
Thanks,
Craig
I like this. I would personally like to see this behaviour deprecated
and removed in 8.0
You specifically mention that counting scalars is unaffected, is there
a legitimate use-case for being able to use count() on them?
I'd say using count() on a string or an int also constitutes a hidden
bug, as it also always returns 1 regardless of the value.
You specifically mention that counting scalars is unaffected, is there
a legitimate use-case for being able to usecount()on them?I'd say using
count()on a string or an int also constitutes a hidden
bug, as it also always returns 1 regardless of the value.
I agree, and I'm happy to include scalars in the RFC if that's desired by
the majority.
Calling count() on a scalar is likely to cause a warning at some point, so
it's not as "hidden" as the iterator example:
if (count($string) > 0) {
foreach ($string as $value) {
You specifically mention that counting scalars is unaffected, is there
a legitimate use-case for being able to usecount()on them?I'd say using
count()on a string or an int also constitutes a hidden
bug, as it also always returns 1 regardless of the value.I agree, and I'm happy to include scalars in the RFC if that's desired by
the majority.Calling
count()on a scalar is likely to cause a warning at some point, so
it's not as "hidden" as the iterator example:if (count($string) > 0) {
foreach ($string as $value) {
A confounding factor is that count() has an alias sizeof() and for people
coming from a C-like background it is quite natural to try to apply
sizeof() to a string in order to get its length. This will silently "work",
but return a meaningless result.
So, definitely +1 on a warning for all non-array non-Countable cases.
Nikita
A confounding factor is that
count()has an aliassizeof()and for people
coming from a C-like background it is quite natural to try to apply
sizeof()to a string in order to get its length. This will silently "work",
but return a meaningless result.So, definitely +1 on a warning for all non-array non-Countable cases.
Ok, but if we're saying that count("string") === 1 is no longer a feature
then presumably it makes sense to deprecate in 7.2 and remove in 8.0?
Just raising a warning but still providing the feature feels weird to me
Hi,
I'd like to propose the introduction of warning when counting objects that
can't be counted.The default behaviour is to return 1 for these objects, which can be
misleading and hide bugs when attempting to count iterable objects (eg
Generators). Adding a warning would alert developers to the issue
FTR, this also applies to certain non-object types too, see
https://3v4l.org/jqntq :
<?php
var_dump(count("string"));
var_dump(count(true));
var_dump(count(false));
var_dump(count(4321));
int(1)
int(1)
int(1)
int(1)
I was hit by this bug just last week :-(
- Markus
I've updated the RFC now to take the deprecation route, and included
counting scalars:
https://wiki.php.net/rfc/counting_non_countables
The only remaining issue is what to do about handling count(null)
I think this should be deprecated too, but as it's the only case that
returns zero I wasn't sure whether other folk felt differently?
I've updated the RFC now to take the deprecation route, and included
counting scalars:https://wiki.php.net/rfc/counting_non_countables
The only remaining issue is what to do about handling count(null)
I think this should be deprecated too, but as it's the only case that
returns zero I wasn't sure whether other folk felt differently?
I'd agree that count(null) has no real benefit, and is probably an error.
I've updated the RFC now to include count(null) which resolves the final
open question.
If there isn't any more feedback I'll open voting in a few days
https://wiki.php.net/rfc/counting_non_countables
Thanks,
Craig
I've updated the RFC now to include count(null) which resolves the final
open question.If there isn't any more feedback I'll open voting in a few days
https://wiki.php.net/rfc/counting_non_countables
Thanks,
Craig
I'm not sure I understand the motivation for throwing a deprecation notice
instead of a warning. In particular, what is the action that will be taken
here in the next major version? I guess we would throw a warning and return
false (instead of 0/1). But is the change of return value from 0/1 to false
really sufficiently worthwhile to go with a deprecation first?
In any case, if you want to go with deprecation, please specify what action
this RFC implies for PHP 8.
Nikita
I'm not sure I understand the motivation for throwing a deprecation notice
instead of a warning. In particular, what is the action that will be taken
here in the next major version? I guess we would throw a warning and return
false (instead of 0/1). But is the change of return value from 0/1 to false
really sufficiently worthwhile to go with a deprecation first?
I don't feel strongly either way, as long as there's some clue that
something's not right.
Is there precedent for adding warnings in a minor? Would there be BC
concerns there?
I wouldn't want an RFC for a warning to fail when people would have voted
for a deprecation.
In any case, if you want to go with deprecation, please specify what
action this RFC implies for PHP 8.
Would it be acceptable for the RFC to state that this has no implications
for PHP 8, and is an indefinite deprecation?
I'm not sure I understand the motivation for throwing a deprecation notice
instead of a warning. In particular, what is the action that will be taken
here in the next major version? I guess we would throw a warning and return
false (instead of 0/1). But is the change of return value from 0/1 to false
really sufficiently worthwhile to go with a deprecation first?I don't feel strongly either way, as long as there's some clue that
something's not right.Is there precedent for adding warnings in a minor? Would there be BC
concerns there?
https://wiki.php.net/rfc/notice-for-non-valid-array-container is
supposed to introduce a new E_WARNING in PHP 7.2.
And yes, there is a (small) BC break, but even E_DEPRECATED might be
regarded as such.
I wouldn't want an RFC for a warning to fail when people would have voted
for a deprecation.
You might consider offering this as voting option.
In any case, if you want to go with deprecation, please specify what
action this RFC implies for PHP 8.Would it be acceptable for the RFC to state that this has no implications
for PHP 8, and is an indefinite deprecation?
In my opinion, an indefinite deprecation doesn't make much sense.
--
Christoph M. Becker
I'm not sure I understand the motivation for throwing a deprecation
notice
instead of a warning. In particular, what is the action that will be
taken
here in the next major version?
https://wiki.php.net/rfc/notice-for-non-valid-array-container is
supposed to introduce a new
E_WARNINGin PHP 7.2.And yes, there is a (small) BC break, but even
E_DEPRECATEDmight be
regarded as such.
Thank you both for the feedback, I've gone with a warning now, I'll put the
RFC to vote in a few days