Skip to content

Commit

Permalink
[Form] Fixed new ArrayChoiceList to compare choices by their values, …
Browse files Browse the repository at this point in the history 
…if enabled
  • Loading branch information
@webmozart
webmozart committed Mar 31, 2015
1 parent e6739bf commit a289deb
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 143 deletions.
60 changes: 42 additions & 18 deletions src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace Symfony\Component\Form\ChoiceList;

use Symfony\Component\Form\Exception\InvalidArgumentException;
use Symfony\Component\Form\Exception\UnexpectedTypeException;

/**
* A list of choices with arbitrary data types.
Expand Down Expand Up @@ -39,33 +39,46 @@ class ArrayChoiceList implements ChoiceListInterface
*/
protected $values = array();

/**
* The callback for creating the value for a choice.
*
* @var callable
*/
protected $valueCallback;

/**
* Creates a list with the given choices and values.
*
* The given choice array must have the same array keys as the value array.
*
* @param array $choices The selectable choices
* @param string[] $values The string values of the choices
*
* @throws InvalidArgumentException If the keys of the choices don't match
* the keys of the values
* @param array $choices The selectable choices
* @param callable $value The callable for creating the value for a
* choice. If `null` is passed, incrementing
* integers are used as values
* @param bool $compareByValue Whether to use the value callback to
* compare choices. If `null`, choices are
* compared by identity
*/
public function __construct(array $choices, array $values)
public function __construct(array $choices, $value = null, $compareByValue = false)
{
$choiceKeys = array_keys($choices);
$valueKeys = array_keys($values);

if ($choiceKeys !== $valueKeys) {
throw new InvalidArgumentException(sprintf(
'The keys of the choices and the values must match. The choice '.
'keys are: "%s". The value keys are: "%s".',
implode('", "', $choiceKeys),
implode('", "', $valueKeys)
));
if (null !== $value && !is_callable($value)) {
throw new UnexpectedTypeException($value, 'null or callable');
}

$this->choices = $choices;
$this->values = array_map('strval', $values);
$this->values = array();
$this->valueCallback = $compareByValue ? $value : null;

if (null === $value) {
$i = 0;
foreach ($this->choices as $key => $choice) {
$this->values[$key] = (string) $i++;
}
} else {
foreach ($choices as $key => $choice) {
$this->values[$key] = (string) call_user_func($value, $choice, $key);
}
}
}

/**
Expand Down Expand Up @@ -116,6 +129,17 @@ public function getValuesForChoices(array $choices)
{
$values = array();

// Use the value callback to compare choices by their values, if present
if ($this->valueCallback) {
$givenValues = array();
foreach ($choices as $key => $choice) {
$givenValues[$key] = (string) call_user_func($this->valueCallback, $choice, $key);
}

return array_intersect($givenValues, $this->values);
}

// Otherwise compare choices by identity
foreach ($choices as $i => $givenChoice) {
foreach ($this->choices as $j => $choice) {
if ($choice !== $givenChoice) {
Expand Down
86 changes: 31 additions & 55 deletions src/Symfony/Component/Form/ChoiceList/ArrayKeyChoiceList.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,14 @@
* @deprecated Added for backwards compatibility in Symfony 2.7, to be removed
* in Symfony 3.0.
*/
class ArrayKeyChoiceList implements ChoiceListInterface
class ArrayKeyChoiceList extends ArrayChoiceList
{
/**
* The selectable choices.
* Whether the choices are used as values.
*
* @var array
* @var bool
*/
private $choices = array();

/**
* The values of the choices.
*
* @var string[]
*/
private $values = array();
private $useChoicesAsValues = false;

/**
* Casts the given choice to an array key.
Expand Down Expand Up @@ -100,63 +93,42 @@ public static function toArrayKey($choice)
* values.
*
* @param array $choices The selectable choices
* @param string[] $values Optional. The string values of the choices
* @param callable $value The callable for creating the value for a
* choice. If `null` is passed, the choices are
* cast to strings and used as values
*
* @throws InvalidArgumentException If the keys of the choices don't match
* the keys of the values or if any of the
* choices is not scalar
*/
public function __construct(array $choices, array $values = array())
public function __construct(array $choices, $value = null)
{
if (empty($values)) {
// The cast to strings happens later
$values = $choices;
} else {
$choiceKeys = array_keys($choices);
$valueKeys = array_keys($values);

if ($choiceKeys !== $valueKeys) {
throw new InvalidArgumentException(
sprintf(
'The keys of the choices and the values must match. The choice '.
'keys are: "%s". The value keys are: "%s".',
implode('", "', $choiceKeys),
implode('", "', $valueKeys)
)
);
}
}

$this->choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
$this->values = array_map('strval', $values);
}
$choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);

/**
* {@inheritdoc}
*/
public function getChoices()
{
return $this->choices;
}
if (null === $value) {
$value = function ($choice) {
return (string) $choice;
};
$this->useChoicesAsValues = true;
}

/**
* {@inheritdoc}
*/
public function getValues()
{
return $this->values;
parent::__construct($choices, $value);
}

/**
* {@inheritdoc}
*/
public function getChoicesForValues(array $values)
{
$values = array_map('strval', $values);
if ($this->useChoicesAsValues) {
$values = array_map('strval', $values);

// If the values are identical to the choices, so we can just return
// them to improve performance a little bit
return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
}

// The values are identical to the choices, so we can just return them
// to improve performance a little bit
return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
return parent::getChoicesForValues($values);
}

/**
Expand All @@ -166,8 +138,12 @@ public function getValuesForChoices(array $choices)
{
$choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);

// The choices are identical to the values, so we can just return them
// to improve performance a little bit
return array_map('strval', array_intersect($choices, $this->choices));
if ($this->useChoicesAsValues) {
// If the choices are identical to the values, we can just return
// them to improve performance a little bit
return array_map('strval', array_intersect($choices, $this->choices));
}

return parent::getValuesForChoices($choices);
}
}
48 changes: 5 additions & 43 deletions src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,6 @@ public function createListFromChoices($choices, $value = null)
throw new UnexpectedTypeException($choices, 'array or \Traversable');
}

if (null !== $value && !is_callable($value)) {
throw new UnexpectedTypeException($value, 'null or callable');
}

if ($choices instanceof \Traversable) {
$choices = iterator_to_array($choices);
}
Expand All @@ -102,29 +98,7 @@ public function createListFromChoices($choices, $value = null)
// in the view only.
self::flatten($choices, $flatChoices);

// If no values are given, use incrementing integers as values
// We can not use the choices themselves, because we don't know whether
// choices can be converted to (duplicate-free) strings
if (null === $value) {
$values = $flatChoices;
$i = 0;

foreach ($values as $key => $value) {
$values[$key] = (string) $i++;
}

return new ArrayChoiceList($flatChoices, $values);
}

// Can't use array_map(), because array_map() doesn't pass the key
// Can't use array_walk(), which ignores the return value of the
// closure
$values = array();
foreach ($flatChoices as $key => $choice) {
$values[$key] = call_user_func($value, $choice, $key);
}

return new ArrayChoiceList($flatChoices, $values);
return new ArrayChoiceList($flatChoices, $value);
}

/**
Expand All @@ -139,10 +113,6 @@ public function createListFromFlippedChoices($choices, $value = null)
throw new UnexpectedTypeException($choices, 'array or \Traversable');
}

if (null !== $value && !is_callable($value)) {
throw new UnexpectedTypeException($value, 'null or callable');
}

if ($choices instanceof \Traversable) {
$choices = iterator_to_array($choices);
}
Expand All @@ -157,20 +127,12 @@ public function createListFromFlippedChoices($choices, $value = null)
// strings or integers, we are guaranteed to be able to convert them
// to strings
if (null === $value) {
$values = array_map('strval', $flatChoices);

return new ArrayKeyChoiceList($flatChoices, $values);
}

// Can't use array_map(), because array_map() doesn't pass the key
// Can't use array_walk(), which ignores the return value of the
// closure
$values = array();
foreach ($flatChoices as $key => $choice) {
$values[$key] = call_user_func($value, $choice, $key);
$value = function ($choice) {
return (string) $choice;
};
}

return new ArrayKeyChoiceList($flatChoices, $values);
return new ArrayKeyChoiceList($flatChoices, $value);
}

/**
Expand Down
45 changes: 44 additions & 1 deletion src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ protected function setUp()

protected function createChoiceList()
{
return new ArrayChoiceList($this->getChoices(), $this->getValues());
$i = 0;

return new ArrayChoiceList($this->getChoices());
}

protected function getChoices()
Expand All @@ -49,4 +51,45 @@ public function testFailIfKeyMismatch()
{
new ArrayChoiceList(array(0 => 'a', 1 => 'b'), array(1 => 'a', 2 => 'b'));
}

public function testCreateChoiceListWithValueCallback()
{
$callback = function ($choice, $key) {
return $key.':'.$choice;
};

$choiceList = new ArrayChoiceList(array(2 => 'foo', 7 => 'bar', 10 => 'baz'), $callback);

$this->assertSame(array(2 => '2:foo', 7 => '7:bar', 10 => '10:baz'), $choiceList->getValues());
$this->assertSame(array(1 => 'foo', 2 => 'baz'), $choiceList->getChoicesForValues(array(1 => '2:foo', 2 => '10:baz')));
$this->assertSame(array(1 => '2:foo', 2 => '10:baz'), $choiceList->getValuesForChoices(array(1 => 'foo', 2 => 'baz')));
}

public function testCompareChoicesByIdentityByDefault()
{
$callback = function ($choice) {
return $choice->value;
};

$obj1 = (object) array('value' => 'value1');
$obj2 = (object) array('value' => 'value2');

$choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback);
$this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2)));
$this->assertSame(array(), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2'))));
}

public function testCompareChoicesWithValueCallbackIfCompareByValue()
{
$callback = function ($choice) {
return $choice->value;
};

$obj1 = (object) array('value' => 'value1');
$obj2 = (object) array('value' => 'value2');

$choiceList = new ArrayChoiceList(array($obj1, $obj2), $callback, true);
$this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => $obj2)));
$this->assertSame(array(2 => 'value2'), $choiceList->getValuesForChoices(array(2 => (object) array('value' => 'value2'))));
}
}
Loading

0 comments on commit a289deb

Please sign in to comment.