Skip to content

Commit

Permalink
[Form] Simplified choice list API
Browse files Browse the repository at this point in the history
  • Loading branch information
webmozart committed Apr 10, 2012
1 parent 2645120 commit 2e07256
Show file tree
Hide file tree
Showing 21 changed files with 106 additions and 302 deletions.
9 changes: 3 additions & 6 deletions UPGRADE-2.1.md
Expand Up @@ -158,14 +158,11 @@ UPGRADE FROM 2.0 to 2.1
`getChoices()` and `getChoicesByValues()`. For the latter two, no
replacement exists.

* The strategy for generating the `id` and `name` HTML attributes for choices
in a choice field has changed.
* The strategy for generating the `id` and `name` HTML attributes for
checkboxes and radio buttons in a choice field has changed.

Instead of appending the choice value, a generated integer is now appended
by default. Take care if your JavaScript relies on the old behavior. If you
can guarantee that your choice values only contain ASCII letters, digits,
colons and underscores, you can restore the old behavior by setting the
`index_strategy` choice field option to `ChoiceList::COPY_CHOICE`.
by default. Take care if your JavaScript relies on that.

* In the choice field type's template, the structure of the `choices` variable
has changed.
Expand Down
Expand Up @@ -319,7 +319,7 @@ protected function createIndex($entity)
protected function createValue($entity)
{
if (count($this->identifier) === 1) {
return current($this->getIdentifierValues($entity));
return (string) current($this->getIdentifierValues($entity));
}

return parent::createValue($entity);
Expand Down
Expand Up @@ -304,7 +304,7 @@ protected function createIndex($model)
protected function createValue($model)
{
if (1 === count($this->identifier)) {
return current($this->getIdentifierValues($model));
return (string) current($this->getIdentifierValues($model));
}

return parent::createValue($model);
Expand Down
Expand Up @@ -32,29 +32,6 @@
*/
class ChoiceList implements ChoiceListInterface
{
/**
* Strategy creating new indices/values by creating a copy of the choice.
*
* This strategy can only be used for index creation if choices are
* guaranteed to only contain ASCII letters, digits and underscores.
*
* It can be used for value creation if choices can safely be cast into
* a (unique) string.
*
* @var integer
*/
const COPY_CHOICE = 0;

/**
* Strategy creating new indices/values by generating a new integer.
*
* This strategy can always be applied, but leads to loss of information
* in the HTML source code.
*
* @var integer
*/
const GENERATE = 1;

/**
* The choices with their indices as keys.
*
Expand Down Expand Up @@ -85,24 +62,6 @@ class ChoiceList implements ChoiceListInterface
*/
private $remainingViews = array();

/**
* The strategy used for creating choice indices.
*
* @var integer
* @see COPY_CHOICE
* @see GENERATE
*/
private $indexStrategy;

/**
* The strategy used for creating choice values.
*
* @var integer
* @see COPY_CHOICE
* @see GENERATE
*/
private $valueStrategy;

/**
* Creates a new choice list.
*
Expand All @@ -115,16 +74,9 @@ class ChoiceList implements ChoiceListInterface
* should match the structure of $choices.
* @param array $preferredChoices A flat array of choices that should be
* presented to the user with priority.
* @param integer $valueStrategy The strategy used to create choice values.
* One of COPY_CHOICE and GENERATE.
* @param integer $indexStrategy The strategy used to create choice indices.
* One of COPY_CHOICE and GENERATE.
*/
public function __construct($choices, array $labels, array $preferredChoices = array(), $valueStrategy = self::GENERATE, $indexStrategy = self::GENERATE)
public function __construct($choices, array $labels, array $preferredChoices = array())
{
$this->valueStrategy = $valueStrategy;
$this->indexStrategy = $indexStrategy;

$this->initialize($choices, $labels, $preferredChoices);
}

Expand Down Expand Up @@ -191,13 +143,6 @@ public function getRemainingViews()
public function getChoicesForValues(array $values)
{
$values = $this->fixValues($values);

// If the values are identical to the choices, we can just return them
// to improve performance a little bit
if (self::COPY_CHOICE === $this->valueStrategy) {
return $this->fixChoices(array_intersect($values, $this->values));
}

$choices = array();

foreach ($values as $j => $givenValue) {
Expand All @@ -222,13 +167,6 @@ public function getChoicesForValues(array $values)
public function getValuesForChoices(array $choices)
{
$choices = $this->fixChoices($choices);

// If the values are identical to the choices, we can just return them
// to improve performance a little bit
if (self::COPY_CHOICE === $this->valueStrategy) {
return $this->fixValues(array_intersect($choices, $this->choices));
}

$values = array();

foreach ($this->choices as $i => $choice) {
Expand Down Expand Up @@ -398,17 +336,15 @@ protected function addChoice(&$bucketForPreferred, &$bucketForRemaining, $choice
$index = $this->createIndex($choice);

if ('' === $index || null === $index || !Form::isValidName((string)$index)) {
throw new InvalidConfigurationException('The choice list index "' . $index . '" is invalid. Please set the choice field option "index_generation" to ChoiceList::GENERATE.');
throw new InvalidConfigurationException('The index "' . $index . '" created by the choice list is invalid. It should be a valid, non-empty Form name.');
}

$value = $this->createValue($choice);

if (!is_scalar($value)) {
throw new InvalidConfigurationException('The choice list value of type "' . gettype($value) . '" should be a scalar. Please set the choice field option "value_generation" to ChoiceList::GENERATE.');
if (!is_string($value)) {
throw new InvalidConfigurationException('The value created by the choice list is of type "' . gettype($value) . '", but should be a string.');
}

// Always store values as strings to facilitate comparisons
$value = $this->fixValue($value);
$view = new ChoiceView($value, $label);

$this->choices[$index] = $this->fixChoice($choice);
Expand Down Expand Up @@ -448,29 +384,23 @@ protected function isPreferred($choice, $preferredChoices)
*/
protected function createIndex($choice)
{
if (self::COPY_CHOICE === $this->indexStrategy) {
return $choice;
}

return count($this->choices);
}

/**
* Creates a new unique value for this choice.
*
* Extension point to change the value strategy.
* By default, an integer is generated since it cannot be guaranteed that
* all values in the list are convertible to (unique) strings. Subclasses
* can override this behaviour if they can guarantee this property.
*
* @param mixed $choice The choice to create a value for
*
* @return integer|string A unique value without character limitations.
* @return string A unique string.
*/
protected function createValue($choice)
{
if (self::COPY_CHOICE === $this->valueStrategy) {
return $choice;
}

return count($this->values);
return (string) count($this->values);
}

/**
Expand Down
Expand Up @@ -14,17 +14,12 @@
/**
* Contains choices that can be selected in a form field.
*
* Each choice has four different properties:
* Each choice has three different properties:
*
* - Choice: The choice that should be returned to the application by the
* choice field. Can be any scalar value or an object, but no
* array.
* - Label: A text representing the choice that is displayed to the user.
* - Index: A uniquely identifying index that should only contain ASCII
* characters, digits and underscores. This index is used to
* identify the choice in the HTML "id" and "name" attributes.
* It is also used as index of the arrays returned by the various
* getters of this class.
* - Value: A uniquely identifying value that can contain arbitrary
* characters, but no arrays or objects. This value is displayed
* in the HTML "value" attribute.
Expand Down
Expand Up @@ -19,8 +19,8 @@
/**
* A choice list for object choices.
*
* Supports generation of choice labels, choice groups, choice values and
* choice indices by calling getters of the object (or associated objects).
* Supports generation of choice labels, choice groups and choice values
* by calling getters of the object (or associated objects).
*
* <code>
* $choices = array($user1, $user2);
Expand Down Expand Up @@ -54,13 +54,6 @@ class ObjectChoiceList extends ChoiceList
*/
private $valuePath;

/**
* The property path used to obtain the choice index.
*
* @var PropertyPath
*/
private $indexPath;

/**
* Creates a new object choice list.
*
Expand All @@ -82,18 +75,14 @@ class ObjectChoiceList extends ChoiceList
* @param string $valuePath A property path pointing to the property used
* for the choice values. If not given, integers
* are generated instead.
* @param string $indexPath A property path pointing to the property used
* for the choice indices. If not given, integers
* are generated instead.
*/
public function __construct($choices, $labelPath = null, array $preferredChoices = array(), $groupPath = null, $valuePath = null, $indexPath = null)
public function __construct($choices, $labelPath = null, array $preferredChoices = array(), $groupPath = null, $valuePath = null)
{
$this->labelPath = $labelPath ? new PropertyPath($labelPath) : null;
$this->groupPath = $groupPath ? new PropertyPath($groupPath) : null;
$this->valuePath = $valuePath ? new PropertyPath($valuePath) : null;
$this->indexPath = $indexPath ? new PropertyPath($indexPath) : null;

parent::__construct($choices, array(), $preferredChoices, self::GENERATE, self::GENERATE);
parent::__construct($choices, array(), $preferredChoices);
}

/**
Expand Down Expand Up @@ -148,27 +137,6 @@ protected function initialize($choices, array $labels, array $preferredChoices)
parent::initialize($choices, $labels, $preferredChoices);
}

/**
* Creates a new unique index for this choice.
*
* If a property path for the index was given at object creation,
* the getter behind that path is now called to obtain a new value.
*
* Otherwise a new integer is generated.
*
* @param mixed $choice The choice to create an index for
* @return integer|string A unique index containing only ASCII letters,
* digits and underscores.
*/
protected function createIndex($choice)
{
if ($this->indexPath) {
return $this->indexPath->getValue($choice);
}

return parent::createIndex($choice);
}

/**
* Creates a new unique value for this choice.
*
Expand All @@ -183,7 +151,7 @@ protected function createIndex($choice)
protected function createValue($choice)
{
if ($this->valuePath) {
return $this->valuePath->getValue($choice);
return (string) $this->valuePath->getValue($choice);
}

return parent::createValue($choice);
Expand Down
Expand Up @@ -28,27 +28,6 @@
* ));
* </code>
*
* The default value generation strategy is `ChoiceList::COPY_CHOICE`, because
* choice values must be scalar, and the choices passed to this choice list
* are guaranteed to be scalar.
*
* The default index generation strategy is `ChoiceList::GENERATE`, so that
* your choices can also contain values that are illegal as indices. If your
* choices are guaranteed to start with a letter, digit or underscore and only
* contain letters, digits, underscores, hyphens and colons, you can set the
* strategy to `ChoiceList::COPY_CHOICE` instead.
*
* <code>
* $choices = array(
* 'creditcard' => 'Credit card payment',
* 'cash' => 'Cash payment',
* );
*
* // value generation: COPY_CHOICE (the default)
* // index generation: COPY_CHOICE (custom)
* $choiceList = new SimpleChoiceList($choices, array(), ChoiceList::COPY_CHOICE, ChoiceList::COPY_CHOICE);
* </code>
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class SimpleChoiceList extends ChoiceList
Expand All @@ -64,15 +43,35 @@ class SimpleChoiceList extends ChoiceList
* key pointing to the nested array.
* @param array $preferredChoices A flat array of choices that should be
* presented to the user with priority.
* @param integer $valueStrategy The strategy used to create choice values.
* One of COPY_CHOICE and GENERATE.
* @param integer $indexStrategy The strategy used to create choice indices.
* One of COPY_CHOICE and GENERATE.
*/
public function __construct(array $choices, array $preferredChoices = array(), $valueStrategy = self::COPY_CHOICE, $indexStrategy = self::GENERATE)
public function __construct(array $choices, array $preferredChoices = array())
{
// Flip preferred choices to speed up lookup
parent::__construct($choices, $choices, array_flip($preferredChoices), $valueStrategy, $indexStrategy);
parent::__construct($choices, $choices, array_flip($preferredChoices));
}

/**
* {@inheritdoc}
*/
public function getChoicesForValues(array $values)
{
$values = $this->fixValues($values);

// The values are identical to the choices, so we can just return them
// to improve performance a little bit
return $this->fixChoices(array_intersect($values, $this->getValues()));
}

/**
* {@inheritdoc}
*/
public function getValuesForChoices(array $choices)
{
$choices = $this->fixChoices($choices);

// The choices are identical to the values, so we can just return them
// to improve performance a little bit
return $this->fixValues(array_intersect($choices, $this->getValues()));
}

/**
Expand Down Expand Up @@ -147,16 +146,21 @@ protected function fixChoice($choice)
return $this->fixIndex($choice);
}


/**
* Converts the choices to valid PHP array keys.
*
* @param array $choices The choices.
*
* @return array Valid PHP array keys.
* {@inheritdoc}
*/
protected function fixChoices(array $choices)
{
return $this->fixIndices($choices);
}

/**
* {@inheritdoc}
*/
protected function createValue($choice)
{
// Choices are guaranteed to be unique and scalar, so we can simply
// convert them to strings
return (string) $choice;
}
}

0 comments on commit 2e07256

Please sign in to comment.