Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #315: Convert null to array for count() #317

Closed
wants to merge 1 commit into from
Closed

Fix #315: Convert null to array for count() #317

wants to merge 1 commit into from

Conversation

swmarc
Copy link

@swmarc swmarc commented Dec 6, 2017

count(): Parameter must be an array or an object
that implements Countable

Signed-off-by: Marcel Saegebarth [email protected]

count(): Parameter must be an array or an object
         that implements Countable

Signed-off-by: Marcel Saegebarth <[email protected]>
@theseer
Copy link
Owner

theseer commented Dec 6, 2017

Thank you for looking into this and providing a possible fix.

Instead of doing a type cast to fix the count call, I think the root cause is that I didn't initialize the property as an array. So the simple fix would be to replace private $data; by private $data = [];

Mind to change the fix accordingly? I'll try to merge it tomorrow.

@swmarc
Copy link
Author

swmarc commented Dec 7, 2017

That was my first thought too, but it could be that returning NULL (no method actually implement that) is may expected instead an empty array.
For instance, key() could return NULL because the property isn't initialized also.

@theseer
Copy link
Owner

theseer commented Dec 7, 2017

So there are two bugs ;)

Returning NULL is a bad idea in pretty much any case. I'll try to look into it over the weekend.

Thanks for pointing it out!

@hemberger hemberger mentioned this pull request Dec 28, 2017
@theseer theseer closed this in 3a62a46 Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants