Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

ValueGenerator constant detection #4238

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 88 additions & 9 deletions library/Zend/Code/Generator/ValueGenerator.php
Expand Up @@ -9,6 +9,8 @@

namespace Zend\Code\Generator;

use Zend\Stdlib\ArrayObject;

class ValueGenerator extends AbstractGenerator
{
/**#@+
Expand Down Expand Up @@ -57,13 +59,19 @@ class ValueGenerator extends AbstractGenerator
* @var array
*/
protected $allowedTypes = null;
/**
* Autodetectable constants
* @var ArrayObject
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using an ArrayObject and not simply an array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
protected $constants = null;

/**
* @param mixed $value
* @param string $type
* @param string $outputMode
* @param mixed $value
* @param string $type
* @param string $outputMode
* @param ArrayObject $constants
*/
public function __construct($value = null, $type = self::TYPE_AUTO, $outputMode = self::OUTPUT_MULTIPLE_LINE)
public function __construct($value = null, $type = self::TYPE_AUTO, $outputMode = self::OUTPUT_MULTIPLE_LINE, ArrayObject $constants = null)
{
if ($value !== null) { // strict check is important here if $type = AUTO
$this->setValue($value);
Expand All @@ -74,6 +82,72 @@ public function __construct($value = null, $type = self::TYPE_AUTO, $outputMode
if ($outputMode !== self::OUTPUT_MULTIPLE_LINE) {
$this->setOutputMode($outputMode);
}
if ($constants !== null) {
$this->constants = $constants;
} else {
$this->constants = new ArrayObject();
}

}

/**
* Init constant list by defined and magic constants
*/
public function initEnvironmentConstants()
{
$constants = array(
'__DIR__',
'__FILE__',
'__LINE__',
'__CLASS__',
'__TRAIT__',
'__METHOD__',
'__FUNCTION__',
'__NAMESPACE__',
'::'
);
$constants = array_merge($constants, array_keys(get_defined_constants()), $this->constants->getArrayCopy());
$this->constants->exchangeArray($constants);
}

/**
* Add constant to list
*
* @param string $constant
*
* @return $this
*/
public function addConstant($constant)
{
$this->constants->append($constant);

return $this;
}

/**
* Delete constant from constant list
*
* @param string $constant
*
* @return bool
*/
public function deleteConstant($constant)
{
if (($index = array_search($constant, $this->constants->getArrayCopy())) !== false) {
$this->constants->offsetUnset($index);
}

return $index !== false;
}

/**
* Return constnat list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/constnat/constant/

*
* @return ArrayObject
*/
public function getConstants()
{
return $this->constants;
}

/**
Expand Down Expand Up @@ -198,6 +272,11 @@ public function getAutoDeterminedType($value)
case 'boolean':
return self::TYPE_BOOLEAN;
case 'string':
foreach($this->constants as $constant) {
if(strpos($value,$constant)!==false) {
return self::TYPE_CONSTANT;
}
}
return self::TYPE_STRING;
case 'double':
case 'float':
Expand Down Expand Up @@ -239,7 +318,7 @@ public function generate()
);
foreach ($rii as $curKey => $curValue) {
if (!$curValue instanceof ValueGenerator) {
$curValue = new self($curValue);
$curValue = new self($curValue, self::TYPE_AUTO, self::OUTPUT_MULTIPLE_LINE, $this->getConstants());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3axap4eHko Did you use ArrayObject for this line?

I don't understand the performance improvements you claim. Non return by reference methods implemented on StdLib\ArrayObject are invoked.

The only thing I see the constants are shared by the instances created in this line, so the only need I see $this->constants must be provided as reference. Alternative solutions may

  • Use PHP native ArrayObject
  • Refactor getConstants for return by reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constants is a very big array, containing about 1k string values. Let's length of every string length is 10, therefore total size of array in memory would be greater then 10KB for one instance of ValueGenerator, that means for 1 property of generated class. So for 100 classes with 100 properties total size of constant arrays would be 10MB instead of 10KB for reference value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also you should keep in mind of allocating memory for every array in method. That 100 classes with 100 properties would be have a 10000 arrays

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So native ArrayObject is suitable for this case and Stdlib version is not required, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stdlib\ArrayObject is a wrapper for backward version capability and nothing more

$rii->getSubIterator()->offsetSet($curKey, $curValue);
}
$curValue->setArrayDepth($rii->getDepth());
Expand Down Expand Up @@ -304,9 +383,9 @@ public function generate()
case self::TYPE_OTHER:
default:
throw new Exception\RuntimeException(sprintf(
'Type "%s" is unknown or cannot be used as property default value.',
get_class($value)
));
'Type "%s" is unknown or cannot be used as property default value.',
get_class($value)
));
}

return $output;
Expand Down Expand Up @@ -353,4 +432,4 @@ public function __toString()
{
return $this->generate();
}
}
}
9 changes: 8 additions & 1 deletion tests/ZendTest/Code/Generator/ValueGeneratorTest.php
Expand Up @@ -72,12 +72,15 @@ public function testPropertyDefaultValueCanHandleComplexArrayOfTypes()
5,
'one' => 1,
'two' => '2',
'constant1' => '__DIR__ . \'/anydir1/anydir2\'',
array(
'baz' => true,
'foo',
'bar',
array(
'baz1',
'baz2',
'constant2' => 'ArrayObject::STD_PROP_LIST',
)
),
new ValueGenerator('PHP_EOL', 'constant')
Expand All @@ -88,19 +91,23 @@ public function testPropertyDefaultValueCanHandleComplexArrayOfTypes()
5,
'one' => 1,
'two' => '2',
'constant1' => __DIR__ . '/anydir1/anydir2',
array(
'baz' => true,
'foo',
'bar',
array(
'baz1',
'baz2'
'baz2',
'constant2' => ArrayObject::STD_PROP_LIST
)
),
PHP_EOL
)
EOS;

$valueGenerator = new ValueGenerator();
$valueGenerator->initEnvironmentConstants();
$valueGenerator->setValue($targetValue);
$generatedTargetSource = $valueGenerator->generate();
$this->assertEquals($expectedSource, $generatedTargetSource);
Expand Down