From b712ac1174461eb4e9ba2e3be3323e92639f978f Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Wed, 22 Oct 2014 19:11:55 +0200 Subject: [PATCH] [PropertyAccess] Simplified code --- .../PropertyAccess/PropertyAccessor.php | 43 +++++++------- .../Tests/Fixtures/TestClass.php | 17 +++--- .../Tests/PropertyAccessorTest.php | 58 +++++++------------ 3 files changed, 49 insertions(+), 69 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 436e345e7f..ac414dd309 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -326,18 +326,18 @@ class PropertyAccessor implements PropertyAccessorInterface throw new NoSuchPropertyException(sprintf('Cannot read property "%s" from an array. Maybe you should write the property path as "[%s]" instead?', $property, $property)); } - $camelProp = $this->camelize($property); + $camelized = $this->camelize($property); $reflClass = new \ReflectionClass($object); - $getter = 'get'.$camelProp; - $getter2 = lcfirst($camelProp); - $isser = 'is'.$camelProp; - $hasser = 'has'.$camelProp; + $getter = 'get'.$camelized; + $getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item) + $isser = 'is'.$camelized; + $hasser = 'has'.$camelized; $classHasProperty = $reflClass->hasProperty($property); if ($reflClass->hasMethod($getter) && $reflClass->getMethod($getter)->isPublic()) { $result[self::VALUE] = $object->$getter(); - } elseif ($this->isMethodAccessible($reflClass, $getter2, 0)) { - $result[self::VALUE] = $object->$getter2(); + } elseif ($this->isMethodAccessible($reflClass, $getsetter, 0)) { + $result[self::VALUE] = $object->$getsetter(); } elseif ($reflClass->hasMethod($isser) && $reflClass->getMethod($isser)->isPublic()) { $result[self::VALUE] = $object->$isser(); } elseif ($reflClass->hasMethod($hasser) && $reflClass->getMethod($hasser)->isPublic()) { @@ -359,7 +359,7 @@ class PropertyAccessor implements PropertyAccessorInterface // we call the getter and hope the __call do the job $result[self::VALUE] = $object->$getter(); } else { - $methods = array($getter, $getter2, $isser, $hasser, '__get'); + $methods = array($getter, $getsetter, $isser, $hasser, '__get'); if ($this->magicCall) { $methods[] = '__call'; } @@ -416,8 +416,8 @@ class PropertyAccessor implements PropertyAccessorInterface } $reflClass = new \ReflectionClass($object); - $plural = $this->camelize($property); - $singulars = (array) StringUtil::singularify($plural); + $camelized = $this->camelize($property); + $singulars = (array) StringUtil::singularify($camelized); if (is_array($value) || $value instanceof \Traversable) { $methods = $this->findAdderAndRemover($reflClass, $singulars); @@ -430,14 +430,14 @@ class PropertyAccessor implements PropertyAccessorInterface } } - $setter = 'set'.$this->camelize($property); - $setter2 = lcfirst($plural); + $setter = 'set'.$camelized; + $getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item) $classHasProperty = $reflClass->hasProperty($property); if ($this->isMethodAccessible($reflClass, $setter, 1)) { $object->$setter($value); - } elseif ($this->isMethodAccessible($reflClass, $setter2, 1)) { - $object->$setter2($value); + } elseif ($this->isMethodAccessible($reflClass, $getsetter, 1)) { + $object->$getsetter($value); } elseif ($this->isMethodAccessible($reflClass, '__set', 2)) { $object->$property = $value; } elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) { @@ -461,7 +461,7 @@ class PropertyAccessor implements PropertyAccessorInterface return '"add'.$singular.'()"/"remove'.$singular.'()", '; }, $singulars)), $setter, - $setter2, + $getsetter, $reflClass->name )); } @@ -529,12 +529,13 @@ class PropertyAccessor implements PropertyAccessorInterface $reflClass = new \ReflectionClass($object); - $setter = 'set'.$this->camelize($property); - $setter2 = lcfirst($this->camelize($property)); + $camelized = $this->camelize($property); + $setter = 'set'.$camelized; + $getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item) $classHasProperty = $reflClass->hasProperty($property); if ($this->isMethodAccessible($reflClass, $setter, 1) - || $this->isMethodAccessible($reflClass, $setter2, 1) + || $this->isMethodAccessible($reflClass, $getsetter, 1) || $this->isMethodAccessible($reflClass, '__set', 2) || ($classHasProperty && $reflClass->getProperty($property)->isPublic()) || (!$classHasProperty && property_exists($object, $property)) @@ -542,11 +543,9 @@ class PropertyAccessor implements PropertyAccessorInterface return true; } - $plural = $this->camelize($property); + $singulars = (array) StringUtil::singularify($camelized); // Any of the two methods is required, but not yet known - $singulars = (array) StringUtil::singularify($plural); - if (null !== $this->findAdderAndRemover($reflClass, $singulars)) { return true; } @@ -563,7 +562,7 @@ class PropertyAccessor implements PropertyAccessorInterface */ private function camelize($string) { - return preg_replace_callback('/(^|_|\.)+(.)/', function ($match) { return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); }, $string); + return strtr(ucwords(strtr($string, array('_' => ' '))), array(' ' => '')); } /** diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php index 6b99251557..5cd605b164 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php @@ -19,7 +19,7 @@ class TestClass private $publicAccessor; private $publicMethodAccessor; - private $publicMethodMutator; + private $publicGetSetter; private $publicAccessorWithDefaultValue; private $publicAccessorWithRequiredAndDefaultValue; private $publicAccessorWithMoreRequiredParameters; @@ -31,7 +31,7 @@ class TestClass $this->publicProperty = $value; $this->publicAccessor = $value; $this->publicMethodAccessor = $value; - $this->publicMethodMutator = $value; + $this->publicGetSetter = $value; $this->publicAccessorWithDefaultValue = $value; $this->publicAccessorWithRequiredAndDefaultValue = $value; $this->publicAccessorWithMoreRequiredParameters = $value; @@ -99,19 +99,18 @@ class TestClass return $this->publicHasAccessor; } - public function publicMethodAccessor() + public function publicGetSetter($value = null) { - return $this->publicMethodAccessor; - } + if (null !== $value) { + $this->publicGetSetter = $value; + } - public function publicMethodMutator($value) - { - $this->publicMethodMutator = $value; + return $this->publicGetSetter; } public function getPublicMethodMutator() { - return $this->publicMethodMutator; + return $this->publicGetSetter; } protected function setProtectedAccessor($value) diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 4ddf10234b..37a131fc98 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -28,26 +28,6 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->propertyAccessor = new PropertyAccessor(); } - public function getValidGetPropertyPaths() - { - return array_merge( - array( - array(new TestClass('Bernhard'), 'publicMethodAccessor', 'Bernhard', 'Bernhard'), - ), - $this->getValidPropertyPaths() - ); - } - - public function getValidSetPropertyPaths() - { - return array_merge( - array( - array(new TestClass('Bernhard'), 'publicMethodMutator', 'Bernhard', 'Bernhard'), - ), - $this->getValidPropertyPaths() - ); - } - public function getPathsWithMissingProperty() { return array( @@ -80,7 +60,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getValidGetPropertyPaths + * @dataProvider getValidPropertyPaths */ public function testGetValue($objectOrArray, $path, $value) { @@ -181,7 +161,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getValidSetPropertyPaths + * @dataProvider getValidPropertyPaths */ public function testSetValue($objectOrArray, $path) { @@ -297,7 +277,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getValidGetPropertyPaths + * @dataProvider getValidPropertyPaths */ public function testIsReadable($objectOrArray, $path) { @@ -365,11 +345,11 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider getValidSetPropertyPaths + * @dataProvider getValidPropertyPaths */ public function testIsWritable($objectOrArray, $path) { - $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path)); } /** @@ -377,7 +357,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase */ public function testIsWritableReturnsFalseIfPropertyNotFound($objectOrArray, $path) { - $this->assertFalse($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + $this->assertFalse($this->propertyAccessor->isWritable($objectOrArray, $path)); } /** @@ -386,7 +366,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase public function testIsWritableReturnsTrueIfIndexNotFound($objectOrArray, $path) { // Non-existing indices can be written. Arrays are created on-demand. - $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path)); } /** @@ -397,42 +377,42 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->propertyAccessor = new PropertyAccessor(false, true); // Non-existing indices can be written even if exceptions are enabled - $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path)); } public function testIsWritableRecognizesMagicSet() { - $this->assertTrue($this->propertyAccessor->isWritable(new TestClassMagicGet('Bernhard'), 'magicProperty', 'Updated')); + $this->assertTrue($this->propertyAccessor->isWritable(new TestClassMagicGet('Bernhard'), 'magicProperty')); } public function testIsWritableDoesNotRecognizeMagicCallByDefault() { - $this->assertFalse($this->propertyAccessor->isWritable(new TestClassMagicCall('Bernhard'), 'magicCallProperty', 'Updated')); + $this->assertFalse($this->propertyAccessor->isWritable(new TestClassMagicCall('Bernhard'), 'magicCallProperty')); } public function testIsWritableRecognizesMagicCallIfEnabled() { $this->propertyAccessor = new PropertyAccessor(true); - $this->assertTrue($this->propertyAccessor->isWritable(new TestClassMagicCall('Bernhard'), 'magicCallProperty', 'Updated')); + $this->assertTrue($this->propertyAccessor->isWritable(new TestClassMagicCall('Bernhard'), 'magicCallProperty')); } - public function testIsWritableThrowsExceptionIfNotObjectOrArray() + public function testNotObjectOrArrayIsNotWritable() { - $this->assertFalse($this->propertyAccessor->isWritable('baz', 'foobar', 'Updated')); + $this->assertFalse($this->propertyAccessor->isWritable('baz', 'foobar')); } - public function testIsWritableThrowsExceptionIfNull() + public function testNullIsNotWritable() { - $this->assertFalse($this->propertyAccessor->isWritable(null, 'foobar', 'Updated')); + $this->assertFalse($this->propertyAccessor->isWritable(null, 'foobar')); } - public function testIsWritableThrowsExceptionIfEmpty() + public function testEmptyIsNotWritable() { - $this->assertFalse($this->propertyAccessor->isWritable('', 'foobar', 'Updated')); + $this->assertFalse($this->propertyAccessor->isWritable('', 'foobar')); } - private function getValidPropertyPaths() + public function getValidPropertyPaths() { return array( array(array('Bernhard', 'Schussek'), '[0]', 'Bernhard'), @@ -451,9 +431,11 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase array(new TestClass('Bernhard'), 'publicAccessorWithRequiredAndDefaultValue', 'Bernhard'), array(new TestClass('Bernhard'), 'publicIsAccessor', 'Bernhard'), array(new TestClass('Bernhard'), 'publicHasAccessor', 'Bernhard'), + array(new TestClass('Bernhard'), 'publicGetSetter', 'Bernhard'), // Methods are camelized array(new TestClass('Bernhard'), 'public_accessor', 'Bernhard'), + array(new TestClass('Bernhard'), '_public_accessor', 'Bernhard'), // Missing indices array(array('index' => array()), '[index][firstName]', null),