-
Notifications
You must be signed in to change notification settings - Fork 27
allow KeyValueContainer objects to be passed as key-value arrays #158
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,9 @@ | |
|
||
namespace Symfony\Cmf\Bundle\SeoBundle\Model; | ||
|
||
use Burgov\Bundle\KeyValueFormBundle\KeyValueContainer; | ||
use Symfony\Cmf\Bundle\SeoBundle\Exception\InvalidArgumentException; | ||
|
||
/** | ||
* This class is a container for the metadata. | ||
* | ||
|
@@ -190,9 +193,9 @@ public function getTitle() | |
/** | ||
* {@inheritDoc} | ||
*/ | ||
public function setExtraProperties(array $extraProperties) | ||
public function setExtraProperties($extraProperties) | ||
{ | ||
$this->extraProperties = $extraProperties; | ||
$this->extraProperties = $this->toArray($extraProperties); | ||
|
||
return $this; | ||
} | ||
|
@@ -226,9 +229,9 @@ public function removeExtraProperty($key) | |
/** | ||
* {@inheritDoc} | ||
*/ | ||
public function setExtraNames(array $extraNames) | ||
public function setExtraNames($extraNames) | ||
{ | ||
$this->extraNames = $extraNames; | ||
$this->extraNames = $this->toArray($extraNames); | ||
|
||
return $this; | ||
} | ||
|
@@ -262,9 +265,9 @@ public function removeExtraName($key) | |
/** | ||
* {@inheritDoc} | ||
*/ | ||
public function setExtraHttp(array $extraHttp) | ||
public function setExtraHttp($extraHttp) | ||
{ | ||
$this->extraHttp = $extraHttp; | ||
$this->extraHttp = $this->toArray($extraHttp); | ||
|
||
return $this; | ||
} | ||
|
@@ -294,4 +297,17 @@ public function removeExtraHttp($key) | |
unset($this->extraHttp[$key]); | ||
} | ||
} | ||
|
||
private function toArray($data) | ||
{ | ||
if ($data instanceof KeyValueContainer) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this requires people to use the BurgovKeyValueFormBundle when using this class, while it is only required when using the form type/admin. I would suggest to add a little check in here to make it optional: if (class_exists('Burgov\Bundle\KeyValueFormBundle\KeyValueContainer') && $data instanceof KeyValueContainer) {
// ...
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wouterj php is sloppy enough to accept a non-existing class in that place. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, indeed... http://3v4l.org/BldOT |
||
$data = $data->toArray(); | ||
} | ||
|
||
if (!is_array($data)) { | ||
throw new InvalidArgumentException(sprintf('Expected either an array or KeyValueContainer, got "%s"', is_object($data) ? getclass($data) : get_type($data))); | ||
} | ||
|
||
return $data; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
|
||
namespace Symfony\Cmf\Bundle\SeoBundle\Model; | ||
|
||
use Burgov\Bundle\KeyValueFormBundle\KeyValueContainer; | ||
|
||
/** | ||
* The interface for the SeoMetadata. | ||
* | ||
|
@@ -79,19 +81,19 @@ public function setTitle($title); | |
public function getTitle(); | ||
|
||
/** | ||
* @param array | ||
* @param array|KeyValueContainer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it OK like this, or should I remove the use statement and use FQDN's for all three methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, you did it the correct way :) |
||
*/ | ||
public function setExtraProperties(array $extraProperties); | ||
public function setExtraProperties($extraProperties); | ||
|
||
/** | ||
* @param array | ||
* @param array|KeyValueContainer | ||
*/ | ||
public function setExtraNames(array $extraNames); | ||
public function setExtraNames($extraNames); | ||
|
||
/** | ||
* @param array | ||
* @param array|KeyValueContainer | ||
*/ | ||
public function setExtraHttp(array $extraHttp); | ||
public function setExtraHttp($extraHttp); | ||
|
||
/** | ||
* @return array | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be an empty line between the namespace and use declarations