Skip to content

Commit

Permalink
[Form] Tweaked the generation of option tags for performance (PHP +20…
Browse files Browse the repository at this point in the history 
…0ms, Twig +50ms)
  • Loading branch information
@webmozart
webmozart committed Jul 21, 2012
1 parent dcfd1eb commit 0aee506
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 82 deletions.
44 changes: 20 additions & 24 deletions Extension/Core/ChoiceList/ChoiceList.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,27 +240,19 @@ public function getIndicesForValues(array $values)
* view objects.
* @param array $bucketForRemaining The bucket where to store the
* non-preferred view objects.
* @param array $choices The list of choices.
* @param array $labels The labels corresponding to the choices.
* @param array $preferredChoices The preferred choices.
* @param array $choices The list of choices.
* @param array $labels The labels corresponding to the choices.
* @param array $preferredChoices The preferred choices.
*
* @throws UnexpectedTypeException If the structure of the $labels array
* does not match the structure of the
* $choices array.
*/
protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices)
protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices)
{
if (!is_array($choices) && !$choices instanceof \Traversable) {
throw new UnexpectedTypeException($choices, 'array or \Traversable');
}

// Add choices to the nested buckets
foreach ($choices as $group => $choice) {
if (is_array($choice)) {
if (!is_array($labels)) {
throw new UnexpectedTypeException($labels, 'array');
}

// Don't do the work if the array is empty
if (count($choice) > 0) {
$this->addChoiceGroup(
Expand Down Expand Up @@ -292,11 +284,11 @@ protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choic
* view objects.
* @param array $bucketForRemaining The bucket where to store the
* non-preferred view objects.
* @param array $choices The list of choices in the group.
* @param array $labels The labels corresponding to the choices in the group.
* @param array $preferredChoices The preferred choices.
* @param array $choices The list of choices in the group.
* @param array $labels The labels corresponding to the choices in the group.
* @param array $preferredChoices The preferred choices.
*/
protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices)
protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices)
{
// If this is a choice group, create a new level in the choice
// key hierarchy
Expand All @@ -323,13 +315,15 @@ protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemai
/**
* Adds a new choice.
*
* @param array $bucketForPreferred The bucket where to store the preferred
* view objects.
* @param array $bucketForRemaining The bucket where to store the
* non-preferred view objects.
* @param mixed $choice The choice to add.
* @param string $label The label for the choice.
* @param array $preferredChoices The preferred choices.
* @param array $bucketForPreferred The bucket where to store the preferred
* view objects.
* @param array $bucketForRemaining The bucket where to store the
* non-preferred view objects.
* @param mixed $choice The choice to add.
* @param string $label The label for the choice.
* @param array $preferredChoices The preferred choices.
*
* @throws InvalidConfigurationException If no valid value or index could be created.
*/
protected function addChoice(&$bucketForPreferred, &$bucketForRemaining, $choice, $label, array $preferredChoices)
{
Expand Down Expand Up @@ -366,8 +360,10 @@ protected function addChoice(&$bucketForPreferred, &$bucketForRemaining, $choice
*
* @param mixed $choice The choice to test.
* @param array $preferredChoices An array of preferred choices.
*
* @return Boolean Whether the choice is preferred.
*/
protected function isPreferred($choice, $preferredChoices)
protected function isPreferred($choice, array $preferredChoices)
{
return false !== array_search($choice, $preferredChoices, true);
}
Expand Down
6 changes: 4 additions & 2 deletions Extension/Core/ChoiceList/SimpleChoiceList.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public function getValuesForChoices(array $choices)
*
* @see parent::addChoices
*/
protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices)
protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices)
{
// Add choices to the nested buckets
foreach ($choices as $choice => $label) {
Expand Down Expand Up @@ -126,8 +126,10 @@ protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choic
*
* @param mixed $choice The choice to test.
* @param array $preferredChoices An array of preferred choices.
*
* @return Boolean Whether the choice is preferred.
*/
protected function isPreferred($choice, $preferredChoices)
protected function isPreferred($choice, array $preferredChoices)
{
// Optimize performance over the default implementation
return isset($preferredChoices[$choice]);
Expand Down
17 changes: 4 additions & 13 deletions FormRenderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,24 +175,15 @@ public function renderBlock($block, array $variables = array())
/**
* {@inheritdoc}
*/
public function isChoiceGroup($choice)
public function isChoiceSelected(ChoiceView $choice, $selectedValue)
{
return is_array($choice) || $choice instanceof \Traversable;
}

/**
* {@inheritdoc}
*/
public function isChoiceSelected(FormView $view, ChoiceView $choice)
{
$value = $view->vars['value'];
$choiceValue = $choice->value;

if (is_array($value)) {
return false !== array_search($choiceValue, $value, true);
if (is_array($selectedValue)) {
return false !== array_search($choiceValue, $selectedValue, true);
}

return $choiceValue === $value;
return $choiceValue === $selectedValue;
}

/**
Expand Down
15 changes: 3 additions & 12 deletions FormRendererInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,24 +146,15 @@ public function renderBlock($block, array $variables = array());
*/
public function renderCsrfToken($intention);

/**
* Returns whether the given choice is a group.
*
* @param mixed $choice A choice
*
* @return Boolean Whether the choice is a group
*/
public function isChoiceGroup($choice);

/**
* Returns whether the given choice is selected.
*
* @param FormView $view The view of the choice field
* @param ChoiceView $choice The choice to check
* @param ChoiceView $choice The choice to check.
* @param string|array $selectedValue The selected value(s).
*
* @return Boolean Whether the choice is selected
*/
public function isChoiceSelected(FormView $view, ChoiceView $choice);
public function isChoiceSelected(ChoiceView $choice, $selectedValue);

/**
* Makes a technical name human readable.
Expand Down
32 changes: 1 addition & 31 deletions Tests/FormRendererTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,34 +40,6 @@ protected function setUp()
$this->renderer = new FormRenderer($this->engine, $this->csrfProvider);
}

public function isChoiceGroupProvider()
{
return array(
array(false, 0),
array(false, '0'),
array(false, '1'),
array(false, 1),
array(false, ''),
array(false, null),
array(false, true),

array(true, array()),
);
}

/**
* @dataProvider isChoiceGroupProvider
*/
public function testIsChoiceGroup($expected, $value)
{
$this->assertSame($expected, $this->renderer->isChoiceGroup($value));
}

public function testIsChoiceGroupPart2()
{
$this->assertTrue($this->renderer->isChoiceGroup(new \SplFixedArray(1)));
}

public function isChoiceSelectedProvider()
{
// The commented cases should not be necessary anymore, because the
Expand Down Expand Up @@ -96,10 +68,8 @@ public function isChoiceSelectedProvider()
*/
public function testIsChoiceSelected($expected, $choice, $value)
{
$view = new FormView();
$view->vars['value'] = $value;
$choice = new ChoiceView($choice, $choice . ' label');

$this->assertSame($expected, $this->renderer->isChoiceSelected($view, $choice));
$this->assertSame($expected, $this->renderer->isChoiceSelected($choice, $value));
}
}

0 comments on commit 0aee506

Please sign in to comment.