Everything

php

Home » Project » Newsgroups » php.internals » [RFC] Counting of non-countable objects »

[RFC] Counting of non-countable objects

php.internals

Craig Duncan (PHP)

9 years ago

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

Leigh

9 years ago

On 4 October 2016 at 10:32, Craig Duncan <php@duncanc.co.uk> wrote:

> 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

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.

Craig Duncan (PHP)

9 years ago

On 4 October 2016 at 11:17, Leigh <leight@gmail.com> wrote:

> 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.
>

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) {

Nikita Popov

9 years ago

On Tue, Oct 4, 2016 at 12:29 PM, Craig Duncan <php@duncanc.co.uk> wrote:

> On 4 October 2016 at 11:17, Leigh <leight@gmail.com> wrote:
>
> > 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.
> >
>
> 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

Craig Duncan (PHP)

9 years ago

On 4 October 2016 at 18:10, Nikita Popov <nikita.ppv@gmail.com> wrote:

>
> 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.
>
>

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

Markus Fischer

9 years ago

Hi,

On 04.10.16 11:32, Craig Duncan wrote:

> 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

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

Craig Duncan (PHP)

9 years ago

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?

Leigh

9 years ago

On 11 October 2016 at 10:49, Craig Duncan <php@duncanc.co.uk> wrote:

> 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.

Craig Duncan (PHP)

9 years ago

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

Nikita Popov

9 years ago

On Mon, Oct 17, 2016 at 10:50 PM, Craig Duncan <php@duncanc.co.uk> wrote:

> 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

Craig Duncan (PHP)

9 years ago

On 17 October 2016 at 21:57, Nikita Popov <nikita.ppv@gmail.com> wrote:

>
>
> 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?

Christoph Becker

9 years ago

On 17.10.2016 at 23:09, Craig Duncan wrote:

> On 17 October 2016 at 21:57, Nikita Popov <nikita.ppv@gmail.com> wrote:
>>
>> 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

Craig Duncan (PHP)

9 years ago

On 17 October 2016 at 21:57, Nikita Popov <nikita.ppv@gmail.com> wrote:

> > 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?
>

On 18 October 2016 at 12:53, Christoph M. Becker <cmbecker69@gmx.de> wrote:
<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.
>

Thank you both for the feedback, I've gone with a warning now, I'll put the
RFC to vote in a few days

https://wiki.php.net/rfc/counting_non_countables