Bugfix/array should be valid datasource for resultset #255
Conversation
test/ResultSet/ReturnType.php
Outdated
| @@ -0,0 +1,13 @@ | |||
| <?php | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
You should remove the extra new lines, they make the build fail but it's also messy
test/ResultSet/ReturnType.php
Outdated
| namespace ZendTest\Db\ResultSet; | ||
|
|
||
|
|
||
| class ReturnType |
There was a problem hiding this comment.
If I'm not mistaken you've created the ReturnType class to force a null to be returned from exchangeArray. You might want to add that in the class' docblock to make that clear to other developers.
…urnType test class
|
Thanks @waltertamboer for the prompt review. I have updated the ReturnType test class. |
test/ResultSet/ReturnType.php
Outdated
| * This class is just here to be able to provide the simplest possible custom return type | ||
| * for a ResultSet when calling ResultSet::setArrayObjectPrototype or using the constructor argument. | ||
| * | ||
| * Class ReturnType |
There was a problem hiding this comment.
This is quite obvious ;-)
@weierophinney Not sure about the package tag? Does ZF have a standard for this?
|
|
||
| $returnType = new ReturnType(); | ||
| $dataSource = [$returnType]; | ||
| $this->resultSet->setArrayObjectPrototype($returnType); |
There was a problem hiding this comment.
I think it can use :
$dataSource = [new ArrayObject([], ArrayObject::ARRAY_AS_PROPS)];
$this->resultSet->setArrayObjectPrototype($dataSource);so no need to have ReturnType.php file.
There was a problem hiding this comment.
Will that return null when exchangeArray is called?
There was a problem hiding this comment.
Thanks for your feedback. I don't think your suggestion covers the use case, while using ArrayObject obfuscates what I'm trying to prove, as ArrayObject is used all over ResultSet and it seems to know very well how to handle it. Problems arise when you are not using an ArrayObject, so I would rather not venture into using it. Also, when I try it as you suggested, the test case testCanProvideArrayAsDataSource fails with Object must be of type ArrayObject, or at least implement exchangeArray.
There was a problem hiding this comment.
@waltertamboer It will return null. Why?
There was a problem hiding this comment.
@bartmcleod the object required for it is an object that need to has exhangeArray(), while ArrayObject already has it. I tried with ArrayObject and working fine at mine:
There was a problem hiding this comment.
oops, sorry, wrong variable, will retry
There was a problem hiding this comment.
should be ok with:
$returnType = new ArrayObject([], ArrayObject::ARRAY_AS_PROPS);
$dataSource = [$returnType];
$this->resultSet->setArrayObjectPrototype($returnType);
$this->resultSet->initialize($dataSource);There was a problem hiding this comment.
@bartmcleod I created PR to your PR branch https://github.com/bartmcleod/zend-db/pull/1
…tasource-for-resultset use $returnType as ArrayObject for arrayobject prototype
|
@samsonasik thanks for your contribution |
|
@bartmcleod if this is a bugfix why did you send to develop branch? |
|
@ezimuel I haven't contributed in a while. Where should it go? master? Happy to change the target. |
|
@bartmcleod I can do that, no problem. I was just checking if this was a bugfix. Thanks! |
|
@ezimuel Ok, thanks. It's a bugfix. |
|
@bartmcleod I tried to merge in master but it seems there are some issues. Can you try to send the PR against master? Thanks. |
|
@ezimuel I will set a reminder for myself |
|
@bartmcleod I finally rebased the PR and merged in master (and develop). I'm closing the PR. Thanks again for your contribution! |
|
@ezimuel welcome, and thanks for the merge. I had the reminder set for tomorrow morning, can clear that now :-) |
When I upgraded from an older version of Zend\Db I found that certain unit tests in the client project were failing. When a datasource provided to a ResultSet is an array, the ResultSet should work out of the box. It didn't, because current() did not account for a datasource which has entered the system as just an array, while the buffer is flagged -1, when the ResultSet was initialized with such a datasource. So that flag on the buffer can be used to early out current() in such a situation.