merged branch Tobion/proppath (PR #5313)

Commits
-------

79a1257 [Form] removed getPositions from PropertyPathInterface

Discussion
----------

[Form] removed getPositions from PropertyPathInterface

This method was just an implementation detail (that is not even needed as my implementation shows) and should not be part of the public API as it serves no purpose.

---------------------------------------------------------------------------

by fabpot at 2012-08-22T06:19:35Z

ping @bschussek

---------------------------------------------------------------------------

by stof at 2012-08-22T09:11:51Z

what is the performance impact of your implementation compared to the previous one ? the form binding is executing this code thousands times for big forms.

---------------------------------------------------------------------------

by Tobion at 2012-08-22T14:08:39Z

There is none of course.

---------------------------------------------------------------------------

by bschussek at 2012-08-22T15:23:57Z

Looks good to me.
This commit is contained in:
Fabien Potencier 2012-08-22 17:51:51 +02:00
commit 1121ef0716
3 changed files with 21 additions and 63 deletions

View File

@ -24,11 +24,6 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
*/
private $elements = array();
/**
* @var array
*/
private $positions = array();
/**
* @var array
*/
@ -59,7 +54,6 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
{
$path = new PropertyPath($violationPath);
$elements = $path->getElements();
$positions = $path->getPositions();
$data = false;
for ($i = 0, $l = count($elements); $i < $l; ++$i) {
@ -76,7 +70,6 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
}
$this->elements[] = $elements[$i];
$this->positions[] = $positions[$i];
$this->isIndex[] = true;
$this->mapsForm[] = true;
} elseif ('data' === $elements[$i] && $path->isProperty($i)) {
@ -89,7 +82,6 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
}
$this->elements[] = $elements[$i];
$this->positions[] = $positions[$i];
$this->isIndex[] = $path->isIndex($i);
$this->mapsForm[] = false;
$data = true;
@ -102,16 +94,14 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
// Already after the "data" element
// Pick everything as is
$this->elements[] = $elements[$i];
$this->positions[] = $positions[$i];
$this->isIndex[] = $path->isIndex($i);
$this->mapsForm[] = false;
}
}
$this->length = count($this->elements);
$this->pathAsString = $violationPath;
$this->resizeString();
$this->buildString();
}
/**
@ -122,14 +112,6 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
return $this->pathAsString;
}
/**
* {@inheritdoc}
*/
public function getPositions()
{
return $this->positions;
}
/**
* {@inheritdoc}
*/
@ -153,9 +135,8 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
array_pop($parent->elements);
array_pop($parent->isIndex);
array_pop($parent->mapsForm);
array_pop($parent->positions);
$parent->resizeString();
$parent->buildString();
return $parent;
}
@ -242,24 +223,27 @@ class ViolationPath implements \IteratorAggregate, PropertyPathInterface
}
/**
* Resizes the string representation to match the number of elements.
* Builds the string representation from the elements.
*/
private function resizeString()
private function buildString()
{
$lastIndex = $this->length - 1;
$this->pathAsString = '';
$data = false;
if ($lastIndex < 0) {
$this->pathAsString = '';
} else {
// +1 for the dot/opening bracket
$length = $this->positions[$lastIndex] + strlen($this->elements[$lastIndex]) + 1;
if ($this->isIndex[$lastIndex]) {
// +1 for the closing bracket
++$length;
foreach ($this->elements as $index => $element) {
if ($this->mapsForm[$index]) {
$this->pathAsString .= ".children[$element]";
} elseif (!$data) {
$this->pathAsString .= '.data' . ($this->isIndex[$index] ? "[$element]" : ".$element");
$data = true;
} else {
$this->pathAsString .= $this->isIndex[$index] ? "[$element]" : ".$element";
}
}
$this->pathAsString = substr($this->pathAsString, 0, $length);
if ('' !== $this->pathAsString) {
// remove leading dot
$this->pathAsString = substr($this->pathAsString, 1);
}
}
}

View File

@ -65,12 +65,6 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
*/
private $pathAsString;
/**
* Positions where the individual elements start in the string representation
* @var array
*/
private $positions;
/**
* Constructs a property path from a string.
*
@ -89,7 +83,6 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
$this->length = $propertyPath->length;
$this->isIndex = $propertyPath->isIndex;
$this->pathAsString = $propertyPath->pathAsString;
$this->positions = $propertyPath->positions;
return;
}
@ -109,8 +102,6 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
$pattern = '/^(([^\.\[]+)|\[([^\]]+)\])(.*)/';
while (preg_match($pattern, $remaining, $matches)) {
$this->positions[] = $position;
if ('' !== $matches[2]) {
$element = $matches[2];
$this->isIndex[] = false;
@ -136,7 +127,7 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
$pattern = '/^(\.(\w+)|\[([^\]]+)\])(.*)/';
}
if (!empty($remaining)) {
if ('' !== $remaining) {
throw new InvalidPropertyPathException(sprintf(
'Could not parse property path "%s". Unexpected token "%s" at position %d',
$propertyPath,
@ -156,14 +147,6 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
return $this->pathAsString;
}
/**
* {@inheritdoc}
*/
public function getPositions()
{
return $this->positions;
}
/**
* {@inheritdoc}
*/
@ -184,11 +167,10 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
$parent = clone $this;
--$parent->length;
$parent->pathAsString = substr($parent->pathAsString, 0, $parent->positions[$parent->length]);
$parent->pathAsString = substr($parent->pathAsString, 0, max(strrpos($parent->pathAsString, '.'), strrpos($parent->pathAsString, '[')));
array_pop($parent->elements);
array_pop($parent->singulars);
array_pop($parent->isIndex);
array_pop($parent->positions);
return $parent;
}

View File

@ -24,15 +24,7 @@ interface PropertyPathInterface extends \Traversable
public function __toString();
/**
* Returns the positions at which the elements of the path
* start in the string.
*
* @return array The string offsets of the elements.
*/
public function getPositions();
/**
* Returns the length of the property path.
* Returns the length of the property path, i.e. the number of elements.
*
* @return integer The path length.
*/