added missing return $this to setValue method. #7114
Conversation
added return $this to the setValue method to implement fluid interface like other setters in this class.
|
Requires a test case to prevent regressions. |
| @@ -112,6 +112,7 @@ public function setValue($value) | |||
| $this->yearElement->setValue($value['year']); | |||
| $this->monthElement->setValue($value['month']); | |||
| $this->dayElement->setValue($value['day']); | |||
| return $this; | |||
There was a problem hiding this comment.
Docblock of the method should also be modified
|
ok, will do that shortly. |
|
DocBlock has been updated. For the regression test, as this method had no previous return value I'm not sure what to test exactly. This update cannot impact existing functionality and all current test cases are passing. |
|
@matwright the test should simply assert that the return value is the same of the object the method is called upon |
| @@ -89,7 +89,7 @@ public function getDayAttributes() | |||
| /** | |||
| * @param string|array|\ArrayAccess|PhpDateTime $value | |||
| * @throws \Zend\Form\Exception\InvalidArgumentException | |||
| * @return void|\Zend\Form\Element | |||
| * @return DateSelect | |||
There was a problem hiding this comment.
@return self imo for consistency.
There was a problem hiding this comment.
@samsonasik The other setters in this class use @return DateSelect
would you like me to update this and the other setter docblocks to show return self ?
There was a problem hiding this comment.
change only for this method i think because it is new introduced docblock. there is inconsistencies about it in other files too. i created PR in the past but can't finish to update all.
|
you can use : $this->assertInstanceOf('Zend\Form\Element\DateSelect', $element->setValue('now')); |
the object upon which the method was called.
|
@samsonasik should be |
| public function testValueSetterReturnsSameObjectType() | ||
| { | ||
| $element = new DateSelectElement(); | ||
| $this->assertInstanceOf(get_class($element), $element->setValue('hello world')); |
There was a problem hiding this comment.
setValue('hello world') must be end up with Exception because of this https://github.com/matwright/zf2/blob/master/library/Zend/Form/Element/DateSelect.php#L97-L101
There was a problem hiding this comment.
thanks @samsonasik just updated that to a valid date string
|
👍 |
|
Merged, thanks @matwright and @samsonasik! |
added return $this to the setValue method to implement fluid interface like other setters in this class.