Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

added missing return $this to setValue method. #7114

Closed
wants to merge 5 commits into from
Closed

added missing return $this to setValue method. #7114

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b44fdda
added missing return $this to setValue method.
matwright Jan 9, 2015
754aa7f
Updated method Docblock return value
matwright Jan 9, 2015
15d2f1e
Added test to assert that getValue returns the same type of object as
matwright Jan 9, 2015
448c1ad
updated test to use assertSame
matwright Jan 9, 2015
792bad2
change docblock to use @return self
matwright Jan 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion library/Zend/Form/Element/DateSelect.php
View file
Open in desktop
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@return self imo for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

*/
public function setValue($value)
{
Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docblock of the method should also be modified

}

/**
Expand Down
6 changes: 6 additions & 0 deletions tests/ZendTest/Form/Element/DateSelectTest.php
View file
Open in desktop
Expand Up @@ -83,4 +83,10 @@ public function testConstructAcceptsDayAttributes()
$dayAttributes = $sut->getDayAttributes();
$this->assertEquals('test', $dayAttributes['class']);
}

public function testValueSetterReturnsSameObjectType()
{
$element = new DateSelectElement();
$this->assertInstanceOf(get_class($element), $element->setValue('hello world'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @samsonasik just updated that to a valid date string

}
}