From 85eec7ae66d3a764620565dd9a3820d64b938ec1 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 11 Jan 2012 17:14:53 -0600 Subject: [PATCH 01/11] Performance improvements/simplification of CallbackHandler - Do not perform lazy instantiation for callbacks - DO validate that static callbacks are actually valid - Use call_user_func vs _array variant for small argument sets - Up to 3 arguments - In PHP 5.4, simply call the callable directly in these cases - Rename "options" to "metadata" - Do not accept $event as first argument; do allow passing it as metadata --- src/CallbackHandler.php | 263 +++++++++++-------------- test/CallbackHandlerTest.php | 55 +++--- test/SignalHandlers/InstanceMethod.php | 2 +- 3 files changed, 145 insertions(+), 175 deletions(-) diff --git a/src/CallbackHandler.php b/src/CallbackHandler.php index 19f09b937..d4db5744b 100644 --- a/src/CallbackHandler.php +++ b/src/CallbackHandler.php @@ -24,6 +24,7 @@ namespace Zend\Stdlib; use Closure, + ReflectionClass, WeakRef; /** @@ -45,11 +46,6 @@ class CallbackHandler */ protected $callback; - /** - * @var string Event to which this handle is subscribed - */ - protected $event; - /** * Until callback has been validated, mark as invalid * @var bool @@ -57,10 +53,10 @@ class CallbackHandler protected $isValidCallback = false; /** - * Callback options, if any + * Callback metadata, if any * @var array */ - protected $options; + protected $metadata; /** * Constructor @@ -70,10 +66,9 @@ class CallbackHandler * @param array $options Options used by the callback handler (e.g., priority) * @return void */ - public function __construct($event, $callback, array $options = array()) + public function __construct($callback, array $metadata = array()) { - $this->event = $event; - $this->options = $options; + $this->metadata = $metadata; $this->registerCallback($callback); } @@ -93,12 +88,19 @@ public function __construct($event, $callback, array $options = array()) */ protected function registerCallback($callback) { - // If pecl/weakref is not installed, simply register it - if (!class_exists('WeakRef', false)) { + if (!is_callable($callback)) { + throw new Exception\InvalidCallbackException('Invalid callback provided; not callable'); + } + + // If pecl/weakref is not installed, simply store the callback and return + if (!class_exists('WeakRef')) { + $this->validateCallback($callback); $this->callback = $callback; return; } + // If WeakRef exists, we want to use it. + // If we have a non-closure object, pass it to WeakRef, and then // register it. if (is_object($callback) && !$callback instanceof Closure) { @@ -117,6 +119,7 @@ protected function registerCallback($callback) // If we have an array callback, and the first argument is not an // object, register as-is if (!is_object($target)) { + $this->validateCallback($callback); $this->callback = $callback; return; } @@ -127,16 +130,6 @@ protected function registerCallback($callback) $this->callback = array($target, $method); } - /** - * Get event to which handler is subscribed - * - * @return string - */ - public function getEvent() - { - return $this->event; - } - /** * Retrieve registered callback * @@ -145,28 +138,31 @@ public function getEvent() */ public function getCallback() { - if (!$this->isValid()) { - throw new Exception\InvalidCallbackException('Invalid callback provided; not callable'); - } - $callback = $this->callback; + + // String callbacks -- simply return if (is_string($callback)) { return $callback; } + // WeakRef callbacks -- pull it out of the object and return it if ($callback instanceof WeakRef) { return $callback->get(); } + // Non-WeakRef object callback -- return it if (is_object($callback)) { return $callback; } + // Array callback with WeakRef object -- retrieve the object first, and + // then return list($target, $method) = $callback; if ($target instanceof WeakRef) { return array($target->get(), $method); } + // Otherwise, return it return $callback; } @@ -179,187 +175,154 @@ public function getCallback() public function call(array $args = array()) { $callback = $this->getCallback(); - return call_user_func_array($callback, $args); + + $isPhp54 = version_compare(PHP_VERSION, '5.4.0rc1', '>='); + + // Minor performance tweak; use call_user_func() until > 3 arguments + // reached + switch (count($args)) { + case 0: + if ($isPhp54) { + return $callback(); + } + return call_user_func($callback); + case 1: + if ($isPhp54) { + return $callback(array_shift($args)); + } + return call_user_func($callback, array_shift($args)); + case 2: + $arg1 = array_shift($args); + $arg2 = array_shift($args); + if ($isPhp54) { + return $callback($arg1, $arg2); + } + return call_user_func($callback, $arg1, $arg2); + case 3: + $arg1 = array_shift($args); + $arg2 = array_shift($args); + $arg3 = array_shift($args); + if ($isPhp54) { + return $callback($arg1, $arg2, $arg3); + } + return call_user_func($callback, $arg1, $arg2, $arg3); + default: + return call_user_func_array($callback, $args); + } } /** - * Get all callback options + * Get all callback metadata * * @return array */ - public function getOptions() + public function getMetadata() { - return $this->options; + return $this->metadata; } /** - * Retrieve a single option + * Retrieve a single metadatum * * @param string $name * @return mixed */ - public function getOption($name) + public function getMetadatum($name) { - if (array_key_exists($name, $this->options)) { - return $this->options[$name]; + if (array_key_exists($name, $this->metadata)) { + return $this->metadata[$name]; } return null; } /** - * Is the composed callback valid? + * Validate a callback * - * Typically, this method simply checks to see if we have a valid callback. - * In a few situations, it does more. + * Attempts to ensure that the provided callback is actually callable. * - * * If we have a string callback, we pass execution to - * {@link validateStringCallback()}. - * * If we have an object callback, we test to see if that object is a - * WeakRef {@see http://pecl.php.net/weakref}. If so, we return the value - * of its valid() method. Otherwise, we return the result of is_callable(). - * * If we have a callback array with a string in the first position, we - * pass execution to {@link validateArrayCallback()}. - * * If we have a callback array with an object in the first position, we - * test to see if that object is a WeakRef (@see http://pecl.php.net/weakref). - * If so, we return the value of its valid() method. Otherwise, we return - * the result of is_callable() on the callback. + * Interestingly, given a class "Foo" with a non-static method "bar", + * is_callable() still returns true for the following callbacks: * - * WeakRef is used to allow listeners to go out of scope. This functionality - * is turn-key if you have pecl/weakref installed; otherwise, you will have - * to manually remove listeners before destroying an object referenced in a - * listener. + * - array('Foo', 'bar') + * - 'Foo::bar' * + * even though passing these to call_user_func*() will raise an error. + * + * @param string|array|object $callback * @return bool + * @throws Exception\InvalidCallbackException */ - public function isValid() + protected function validateCallback($callback) { - // If we've already tested this, we can move on. Note: if a callback - // composes a WeakRef, this will never get set, and thus result in - // validation on each call. - if ($this->isValidCallback) { - return $this->callback; + if (is_array($callback)) { + return $this->validateArrayCallback($callback); } - - $callback = $this->callback; - if (is_string($callback)) { return $this->validateStringCallback($callback); } + return true; + } - if ($callback instanceof WeakRef) { - return $callback->valid(); - } - - if (is_object($callback) && is_callable($callback)) { - $this->isValidCallback = true; + /** + * Validate an array callback + * + * If the first argument of the array is an object, simply returns true. + * Otherwise, passes the array arguments off to validateStaticMethod(). + * + * @param array $callback + * @return true + * @throws Exception\InvalidCallbackException + */ + protected function validateArrayCallback($callback) + { + list($target, $method) = $callback; + if (is_object($target)) { return true; } - if (!is_array($callback)) { - return false; - } - - list($target, $method) = $callback; - if ($target instanceof WeakRef) { - if (!$target->valid()) { - return false; - } - $target = $target->get(); - return is_callable(array($target, $method)); - } - return $this->validateArrayCallback($callback); + return $this->validateStaticMethod($target, $method); } /** * Validate a string callback * - * Check first if the string provided is callable. If not see if it is a - * valid class name; if so, determine if the object is invokable. + * If the callback is a function name, simply returns true. If it refers + * to a static method, proxies to validateStaticMethod(). * * @param string $callback - * @return bool + * @return true + * @throws Exception\InvalidCallbackException */ protected function validateStringCallback($callback) { - if (is_callable($callback)) { - $this->isValidCallback = true; + if (!strstr($callback, '::')) { return true; } - if (!class_exists($callback)) { - return false; - } - - // check __invoke before instantiating - if (!method_exists($callback, '__invoke')) { - return false; - } - $object = new $callback(); - - $this->callback = $object; - $this->isValidCallback = true; - return true; + list($target, $method) = explode('::', $callback, 2); + return $this->validateStaticMethod($target, $method); } /** - * Validate an array callback + * Validates that a static callback actually exists * - * @param array $callback + * @param string $target + * @param string $method * @return bool + * @throws Exception\InvalidCallbackException */ - protected function validateArrayCallback(array $callback) + protected function validateStaticMethod($target, $method) { - $context = $callback[0]; - $method = $callback[1]; - - if (is_string($context)) { - // Dealing with a class/method callback, and class provided is a string classname - - if (!class_exists($context)) { - return false; - } - - // We need to determine if we need to instantiate the class first - $r = new \ReflectionClass($context); - if (!$r->hasMethod($method)) { - // Explicit method does not exist - if (!$r->hasMethod('__callStatic') && !$r->hasMethod('__call')) { - return false; - } - - if ($r->hasMethod('__callStatic')) { - // We have a __callStatic defined, so the original callback is valid - $this->isValidCallback = true; - return $callback; - } + $r = new ReflectionClass($target); + if (!$r->hasMethod($method)) { + throw new Exception\InvalidCallbackException('Invalid callback; method does not exist'); + } - // We have __call defined, so we need to instantiate the class - // first, and redefine the callback - $object = new $context(); - $this->callback = array($object, $method); - $this->isValidCallback = true; - return $this->callback; - } - - // Explicit method exists - $rMethod = $r->getMethod($method); - if ($rMethod->isStatic()) { - // Method is static, so original callback is fine - $this->isValidCallback = true; - return $callback; - } - - // Method is an instance method; instantiate object and redefine callback - $object = new $context(); - $this->callback = array($object, $method); - $this->isValidCallback = true; - return $this->callback; - } elseif (is_callable($callback)) { - // The - $this->isValidCallback = true; - return $callback; + $m = $r->getMethod($method); + if (!$m->isStatic()) { + throw new Exception\InvalidCallbackException('Invalid callback; method is not static'); } - return false; + return true; } } diff --git a/test/CallbackHandlerTest.php b/test/CallbackHandlerTest.php index 32a815eae..977601ad9 100644 --- a/test/CallbackHandlerTest.php +++ b/test/CallbackHandlerTest.php @@ -41,77 +41,84 @@ public function setUp() } } - public function testGetEventShouldReturnEvent() + public function testCallbackShouldStoreMetadata() { - $handler = new CallbackHandler('foo', 'rand'); - $this->assertEquals('foo', $handler->getEvent()); + $handler = new CallbackHandler('rand', array('event' => 'foo')); + $this->assertEquals('foo', $handler->getMetadatum('event')); + $this->assertEquals(array('event' => 'foo'), $handler->getMetadata()); } public function testCallbackShouldBeStringIfNoHandlerPassedToConstructor() { - $handler = new CallbackHandler('foo', 'rand'); + $handler = new CallbackHandler('rand'); $this->assertSame('rand', $handler->getCallback()); } public function testCallbackShouldBeArrayIfHandlerPassedToConstructor() { - $handler = new CallbackHandler('foo', array('\\ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test')); - $this->assertSame(array('\\ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test'), $handler->getCallback()); + $handler = new CallbackHandler(array('ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test')); + $this->assertSame(array('ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test'), $handler->getCallback()); } public function testCallShouldInvokeCallbackWithSuppliedArguments() { - $handler = new CallbackHandler('foo', array( $this, 'handleCall' )); + $handler = new CallbackHandler(array( $this, 'handleCall' )); $args = array('foo', 'bar', 'baz'); $handler->call($args); $this->assertSame($args, $this->args); } - public function testPassingInvalidCallbackShouldRaiseInvalidCallbackExceptionDuringCall() + public function testPassingInvalidCallbackShouldRaiseInvalidCallbackExceptionDuringInstantiation() { $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); - $handler = new CallbackHandler('Invokable', 'boguscallback'); - $handler->call(); + $handler = new CallbackHandler('boguscallback'); } public function testCallShouldReturnTheReturnValueOfTheCallback() { - $handler = new CallbackHandler('foo', array('\\ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test')); - if (!is_callable(array('\\ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test'))) { - echo "\nClass exists? " . var_export(class_exists('\\ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback'), 1) . "\n"; + $handler = new CallbackHandler(array('ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test')); + if (!is_callable(array('ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback', 'test'))) { + echo "\nClass exists? " . var_export(class_exists('ZendTest\\Stdlib\\SignalHandlers\\ObjectCallback'), 1) . "\n"; echo "Include path: " . get_include_path() . "\n"; } $this->assertEquals('bar', $handler->call(array())); } - public function testStringCallbackResolvingToClassNameShouldCallViaInvoke() + public function testStringCallbackResolvingToClassDefiningInvokeNameShouldRaiseException() { - $handler = new CallbackHandler('foo', '\\ZendTest\\Stdlib\\SignalHandlers\\Invokable'); - $this->assertEquals('__invoke', $handler->call(), var_export($handler->getCallback(), 1)); + $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); + $handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\Invokable'); } public function testStringCallbackReferringToClassWithoutDefinedInvokeShouldRaiseException() { $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); - $handler = new CallbackHandler('foo', '\\ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod'); + $handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod'); + } + + public function testCallbackConsistingOfStringContextWithNonStaticMethodShouldRaiseException() + { + $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); + $handler = new CallbackHandler(array('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod', 'handler')); $handler->call(); } - public function testCallbackConsistingOfStringContextWithNonStaticMethodShouldInstantiateContext() + public function testStringCallbackConsistingOfNonStaticMethodShouldRaiseException() { - $handler = new CallbackHandler('foo', array( 'ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod', 'callable' )); - $this->assertEquals('callable', $handler->call()); + $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); + $handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod::handler'); + $handler->call(); } - public function testCallbackToClassImplementingOverloadingShouldSucceed() + public function testCallbackToClassImplementingOverloadingButNotInvocableShouldRaiseException() { - $handler = new CallbackHandler('foo', array( '\\ZendTest\\Stdlib\\SignalHandlers\\Overloadable', 'foo' )); - $this->assertEquals('foo', $handler->call()); + $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); + $handler = new CallbackHandler('foo', array( 'ZendTest\\Stdlib\\SignalHandlers\\Overloadable', 'foo' )); } public function testClosureCallbackShouldBeInvokedByCall() { - $handler = new CallbackHandler(null, function () { + $handler = new CallbackHandler(function () { return 'foo'; }); $this->assertEquals('foo', $handler->call()); diff --git a/test/SignalHandlers/InstanceMethod.php b/test/SignalHandlers/InstanceMethod.php index cf06a79b1..393d32710 100644 --- a/test/SignalHandlers/InstanceMethod.php +++ b/test/SignalHandlers/InstanceMethod.php @@ -3,7 +3,7 @@ class InstanceMethod { - public function callable() + public function handler() { return __FUNCTION__; } From 32a8259fe2eba3680670a2cb6355cc75fb7a3acf Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 12 Jan 2012 09:04:46 -0600 Subject: [PATCH 02/11] Implemented __invoke in CallbackHandler - Proxies to call() --- src/CallbackHandler.php | 10 ++++++++++ test/CallbackHandlerTest.php | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/src/CallbackHandler.php b/src/CallbackHandler.php index d4db5744b..d8686838f 100644 --- a/src/CallbackHandler.php +++ b/src/CallbackHandler.php @@ -211,6 +211,16 @@ public function call(array $args = array()) } } + /** + * Invoke as functor + * + * @return mixed + */ + public function __invoke() + { + return $this->call(func_get_args()); + } + /** * Get all callback metadata * diff --git a/test/CallbackHandlerTest.php b/test/CallbackHandlerTest.php index 977601ad9..5b18f77e9 100644 --- a/test/CallbackHandlerTest.php +++ b/test/CallbackHandlerTest.php @@ -124,6 +124,13 @@ public function testClosureCallbackShouldBeInvokedByCall() $this->assertEquals('foo', $handler->call()); } + public function testHandlerShouldBeInvocable() + { + $handler = new CallbackHandler(array($this, 'handleCall')); + $handler('foo', 'bar'); + $this->assertEquals(array('foo', 'bar'), $this->args); + } + public function handleCall() { $this->args = func_get_args(); From 132d5b601819871798cd4e541b305e937540e10a Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 12 Jan 2012 11:15:35 -0600 Subject: [PATCH 03/11] Do not validate callables - If a callback is_callable, then we should allow it. - Developers will get an E_STRICT when invoking non-static methods statically, and an E_FATAL when using $this in such situations; this is up to the developer to debug and handle, not the framework. --- src/CallbackHandler.php | 103 +---------------------------------- test/CallbackHandlerTest.php | 18 ++++-- 2 files changed, 15 insertions(+), 106 deletions(-) diff --git a/src/CallbackHandler.php b/src/CallbackHandler.php index d8686838f..bdb830574 100644 --- a/src/CallbackHandler.php +++ b/src/CallbackHandler.php @@ -46,12 +46,6 @@ class CallbackHandler */ protected $callback; - /** - * Until callback has been validated, mark as invalid - * @var bool - */ - protected $isValidCallback = false; - /** * Callback metadata, if any * @var array @@ -80,8 +74,7 @@ public function __construct($callback, array $metadata = array()) * * If a callback is a functor, or an array callback composing an object * instance, this method will pass the object to a WeakRef instance prior - * to registering the callback. See {@link isValid()} for more information - * on how this affects execution. + * to registering the callback. * * @param callback $callback * @return void @@ -94,7 +87,6 @@ protected function registerCallback($callback) // If pecl/weakref is not installed, simply store the callback and return if (!class_exists('WeakRef')) { - $this->validateCallback($callback); $this->callback = $callback; return; } @@ -119,7 +111,6 @@ protected function registerCallback($callback) // If we have an array callback, and the first argument is not an // object, register as-is if (!is_object($target)) { - $this->validateCallback($callback); $this->callback = $callback; return; } @@ -134,7 +125,6 @@ protected function registerCallback($callback) * Retrieve registered callback * * @return Callback - * @throws Exception\InvalidCallbackException If callback is invalid */ public function getCallback() { @@ -244,95 +234,4 @@ public function getMetadatum($name) } return null; } - - /** - * Validate a callback - * - * Attempts to ensure that the provided callback is actually callable. - * - * Interestingly, given a class "Foo" with a non-static method "bar", - * is_callable() still returns true for the following callbacks: - * - * - array('Foo', 'bar') - * - 'Foo::bar' - * - * even though passing these to call_user_func*() will raise an error. - * - * @param string|array|object $callback - * @return bool - * @throws Exception\InvalidCallbackException - */ - protected function validateCallback($callback) - { - if (is_array($callback)) { - return $this->validateArrayCallback($callback); - } - if (is_string($callback)) { - return $this->validateStringCallback($callback); - } - return true; - } - - /** - * Validate an array callback - * - * If the first argument of the array is an object, simply returns true. - * Otherwise, passes the array arguments off to validateStaticMethod(). - * - * @param array $callback - * @return true - * @throws Exception\InvalidCallbackException - */ - protected function validateArrayCallback($callback) - { - list($target, $method) = $callback; - if (is_object($target)) { - return true; - } - - return $this->validateStaticMethod($target, $method); - } - - /** - * Validate a string callback - * - * If the callback is a function name, simply returns true. If it refers - * to a static method, proxies to validateStaticMethod(). - * - * @param string $callback - * @return true - * @throws Exception\InvalidCallbackException - */ - protected function validateStringCallback($callback) - { - if (!strstr($callback, '::')) { - return true; - } - - list($target, $method) = explode('::', $callback, 2); - return $this->validateStaticMethod($target, $method); - } - - /** - * Validates that a static callback actually exists - * - * @param string $target - * @param string $method - * @return bool - * @throws Exception\InvalidCallbackException - */ - protected function validateStaticMethod($target, $method) - { - $r = new ReflectionClass($target); - if (!$r->hasMethod($method)) { - throw new Exception\InvalidCallbackException('Invalid callback; method does not exist'); - } - - $m = $r->getMethod($method); - if (!$m->isStatic()) { - throw new Exception\InvalidCallbackException('Invalid callback; method is not static'); - } - - return true; - } } diff --git a/test/CallbackHandlerTest.php b/test/CallbackHandlerTest.php index 5b18f77e9..b8acd085d 100644 --- a/test/CallbackHandlerTest.php +++ b/test/CallbackHandlerTest.php @@ -96,18 +96,28 @@ public function testStringCallbackReferringToClassWithoutDefinedInvokeShouldRais $handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod'); } - public function testCallbackConsistingOfStringContextWithNonStaticMethodShouldRaiseException() + public function testCallbackConsistingOfStringContextWithNonStaticMethodShouldNotRaiseExceptionButWillRaiseEStrict() { - $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); $handler = new CallbackHandler(array('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod', 'handler')); + $error = false; + set_error_handler(function ($errno, $errstr) use (&$error) { + $error = true; + }, E_STRICT); $handler->call(); + restore_error_handler(); + $this->assertTrue($error); } - public function testStringCallbackConsistingOfNonStaticMethodShouldRaiseException() + public function testStringCallbackConsistingOfNonStaticMethodShouldNotRaiseExceptionButWillRaiseEStrict() { - $this->setExpectedException('Zend\Stdlib\Exception\InvalidCallbackException'); $handler = new CallbackHandler('ZendTest\\Stdlib\\SignalHandlers\\InstanceMethod::handler'); + $error = false; + set_error_handler(function ($errno, $errstr) use (&$error) { + $error = true; + }, E_STRICT); $handler->call(); + restore_error_handler(); + $this->assertTrue($error); } public function testCallbackToClassImplementingOverloadingButNotInvocableShouldRaiseException() From ca47165ae3a421033e08bb93066180077bccd08e Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Sun, 5 Feb 2012 14:43:18 +0100 Subject: [PATCH 04/11] replaced is_null($foo->$bar()) with $foo->$bar() === null --- src/Options.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index 82c4c59ed..29a53767d 100644 --- a/src/Options.php +++ b/src/Options.php @@ -136,7 +136,7 @@ public function __get($key) public function __isset($key) { $getter = $this->assembleGetterNameFromConfigKey($key); - return !is_null($this->{$getter}()); + return ($this->{$getter}() !== null); } /** From f3e2be4cb23f6598bdf8feb7431763eaf6a48875 Mon Sep 17 00:00:00 2001 From: Ben Scholzen Date: Mon, 5 Mar 2012 23:21:55 +0100 Subject: [PATCH 05/11] Renamed ArrayReplergeRecursive to RecursiveArrayMerge and updated doc blocks --- ...ayReplergeRecursive.php => RecursiveArrayMerge.php} | 10 +++++----- ...geRecursiveTest.php => RecursiveArrayMergeTest.php} | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) rename src/{ArrayReplergeRecursive.php => RecursiveArrayMerge.php} (85%) rename test/{ArrayReplergeRecursiveTest.php => RecursiveArrayMergeTest.php} (87%) diff --git a/src/ArrayReplergeRecursive.php b/src/RecursiveArrayMerge.php similarity index 85% rename from src/ArrayReplergeRecursive.php rename to src/RecursiveArrayMerge.php index edc6c22c5..6d7a05d40 100644 --- a/src/ArrayReplergeRecursive.php +++ b/src/RecursiveArrayMerge.php @@ -22,17 +22,17 @@ /** - * Simple combination of array_merge_recursive and array_replace_recursive. + * Simple class for merging two arrays. * * @category Zend * @package Zend_Stdlib * @copyright Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ -class ArrayReplergeRecursive +abstract class RecursiveArrayMerge { /** - * Merge two arrays together with a mixed merge/replace behaviour. + * Merge two arrays together. * * If an integer key exists in both arrays, the value from the second array * will be appended the the first array. If both values are arrays, they @@ -43,14 +43,14 @@ class ArrayReplergeRecursive * @param array $b * @return array */ - public static function replerge(array $a, array $b) + public static function merge(array $a, array $b) { foreach ($b as $key => $value) { if (array_key_exists($key, $a)) { if (is_int($key)) { $a[] = $value; } elseif (is_array($value) && is_array($a[$key])) { - $a[$key] = self::replerge($a[$key], $value); + $a[$key] = self::merge($a[$key], $value); } else { $a[$key] = $value; } diff --git a/test/ArrayReplergeRecursiveTest.php b/test/RecursiveArrayMergeTest.php similarity index 87% rename from test/ArrayReplergeRecursiveTest.php rename to test/RecursiveArrayMergeTest.php index b23329931..1dc6cf557 100644 --- a/test/ArrayReplergeRecursiveTest.php +++ b/test/RecursiveArrayMergeTest.php @@ -3,9 +3,9 @@ namespace ZendTest\Stdlib; use PHPUnit_Framework_TestCase as TestCase, - Zend\Stdlib\ArrayReplergeRecursive; + Zend\Stdlib\RecursiveArrayMerge; -class ArrayReplergeRecursiveTest extends TestCase +class RecursiveArrayMergeTest extends TestCase { public static function mergeArrays() { @@ -64,8 +64,8 @@ public static function mergeArrays() /** * @dataProvider mergeArrays */ - public function testReplerge($a, $b, $expected) + public function testMerge($a, $b, $expected) { - $this->assertEquals($expected, ArrayReplergeRecursive::replerge($a, $b)); + $this->assertEquals($expected, RecursiveArrayMerge::merge($a, $b)); } } From da399fb05393dc680c6f6cfec6d24a149107a0d1 Mon Sep 17 00:00:00 2001 From: Artur Bodera Date: Tue, 6 Mar 2012 14:04:56 +0100 Subject: [PATCH 06/11] Introduce Stdlib\ArrayTools, refactor other components to use it. - add Stdlib\ArrayTools which combines the following: - Stdlib\RecursiveArrayMerge - Stdlib\IteratorToArray - Stdlib\IsAssocArray - add ArrayTools array testing functionality: - ::isList() - ::isHashTable() - ::hasNumericKeys() - ::hasIntegerKeys() - ::hasStringKeys() - add more exhaustive unit tests for ArrayTools including edge-cases for different types of arrays --- src/ArrayTools.php | 224 +++++++++++++++++++ src/IsAssocArray.php | 31 --- src/IteratorToArray.php | 63 ------ test/ArrayToolsTest.php | 358 +++++++++++++++++++++++++++++++ test/IsAssocArrayTest.php | 79 ------- test/IteratorToArrayTest.php | 90 -------- test/RecursiveArrayMergeTest.php | 71 ------ 7 files changed, 582 insertions(+), 334 deletions(-) create mode 100644 src/ArrayTools.php delete mode 100644 src/IsAssocArray.php delete mode 100644 src/IteratorToArray.php create mode 100644 test/ArrayToolsTest.php delete mode 100644 test/IsAssocArrayTest.php delete mode 100644 test/IteratorToArrayTest.php delete mode 100644 test/RecursiveArrayMergeTest.php diff --git a/src/ArrayTools.php b/src/ArrayTools.php new file mode 100644 index 000000000..f68ba2eff --- /dev/null +++ b/src/ArrayTools.php @@ -0,0 +1,224 @@ + 0; + } + return false; + } + + /** + * Test whether an array contains one or more integer keys + * + * @param mixed $value + * @param bool $allowEmpty Should an empty array() return true + * @return bool + */ + public static function hasIntegerKeys($value, $allowEmpty = false) + { + if (is_array($value)) { + if (count($value) == 0) { + return $allowEmpty; + } + return count(array_filter(array_keys($value), 'is_int')) > 0; + } + return false; + } + + /** + * Test whether an array contains one or more numeric keys. + * + * A numeric key can be one of the following: + * - an integer 1, + * - a string with a number '20' + * - a string with negative number: '-1000' + * - a float: 2.2120, -78.150999 + * - a strings with float: '4000.99999', '-10.10' + * + * @param mixed $value + * @param bool $allowEmpty Should an empty array() return true + * @return bool + */ + public static function hasNumericKeys($value, $allowEmpty = false) + { + if (is_array($value)) { + if (count($value) == 0) { + return $allowEmpty; + } + return count(array_filter(array_keys($value), 'is_numeric')) > 0; + } + return false; + } + + /** + * Test whether an array is a list + * + * A list is a collection of values assigned to continuous integer keys + * starting at 0 and ending at count() - 1. + * + * For example: + * $list = array( 'a','b','c','d' ); + * $list = array( + * 0 => 'foo', + * 1 => 'bar', + * 2 => array( 'foo' => 'baz' ) + * ); + * + * @param mixed $value + * @param bool $allowEmpty Is an empty list a valid list? + * @return bool + */ + public static function isList($value, $allowEmpty = false) + { + if (is_array($value)) { + if (count($value) == 0) { + return $allowEmpty; + } + return (array_values($value) === $value); + } + return false; + } + + /** + * Test whether an array is a hash table. + * + * An array is a hash table if: + * 1. Contains one or more non-integer keys, or + * 2. Integer keys are non-continuous or misaligned (not starting with 0) + * + * For example: + * $hash = array( + * 'foo' => 15, + * 'bar' => false + * ); + * $hash = array( + * 1995 => 'Birth of PHP', + * 2009 => 'PHP 5.3.0', + * 2012 => 'PHP 5.4.0' + * ); + * $hash = array( + * 'formElement, + * 'options' => array( 'debug' => true ) + * ); + * + * @param mixed $value + * @param bool $allowEmpty Is an empty array() a valid hash table? + * @return bool + */ + public static function isHashTable($value, $allowEmpty = false) + { + if (is_array($value)) { + if (count($value) == 0) { + return $allowEmpty; + } + return (array_values($value) !== $value); + } + return false; + } + + /** + * Convert an iterator to an array. + * + * Converts an iterator to an array. The $recursive flag, on by default, + * hints whether or not you want to do so recursively. + * + * @param array|Traversable $iterator The array or Traversable object to convert + * @param bool $recursive Recursively check all nested structures + * @return array + */ + public static function iteratorToArray($iterator, $recursive = true) + { + if (!is_array($iterator) && !$iterator instanceof Traversable) { + throw new Exception\InvalidArgumentException(__METHOD__ . ' expects an array or Traversable object'); + } + + if (!$recursive) { + if (is_array($iterator)) { + return $iterator; + } + + return iterator_to_array($iterator); + } + + if (method_exists($iterator, 'toArray')) { + return $iterator->toArray(); + } + + $array = array(); + foreach ($iterator as $key => $value) { + if (is_scalar($value)) { + $array[$key] = $value; + continue; + } + + if ($value instanceof Traversable) { + $array[$key] = static::iteratorToArray($value, $recursive); + continue; + } + + if (is_array($value)) { + $array[$key] = static::iteratorToArray($value, $recursive); + continue; + } + + $array[$key] = $value; + } + + return $array; + } + + /** + * Merge two arrays together. + * + * If an integer key exists in both arrays, the value from the second array + * will be appended the the first array. If both values are arrays, they + * are merged together, else the value of the second array overwrites the + * one of the first array. + * + * @param array $a + * @param array $b + * @return array + */ + public static function merge(array $a, array $b) + { + foreach ($b as $key => $value) { + if (array_key_exists($key, $a)) { + if (is_int($key)) { + $a[] = $value; + } elseif (is_array($value) && is_array($a[$key])) { + $a[$key] = self::merge($a[$key], $value); + } else { + $a[$key] = $value; + } + } else { + $a[$key] = $value; + } + } + + return $a; + } + + +} diff --git a/src/IsAssocArray.php b/src/IsAssocArray.php deleted file mode 100644 index 7db232153..000000000 --- a/src/IsAssocArray.php +++ /dev/null @@ -1,31 +0,0 @@ -toArray(); - } - - $array = array(); - foreach ($iterator as $key => $value) { - if (is_scalar($value)) { - $array[$key] = $value; - continue; - } - - if ($value instanceof Traversable) { - $array[$key] = static::convert($value, $recursive); - continue; - } - - if (is_array($value)) { - $array[$key] = static::convert($value, $recursive); - continue; - } - - $array[$key] = $value; - } - - return $array; - } -} diff --git a/test/ArrayToolsTest.php b/test/ArrayToolsTest.php new file mode 100644 index 000000000..c92fceeec --- /dev/null +++ b/test/ArrayToolsTest.php @@ -0,0 +1,358 @@ + 'bar' + )), + array(array( + '15', + 'foo' => 'bar', + 'baz' => array('baz') + )), + array(array( + 0 => false, + 2 => null + )), + array(array( + -100 => 'foo', + 100 => 'bar' + )), + array(array( + 1 => 0 + )), + ); + } + + public static function validLists() + { + return array( + array(array(null)), + array(array(true)), + array(array(false)), + array(array(0)), + array(array(-0.9999)), + array(array('string')), + array(array(new stdClass)), + array(array( + 0 => 'foo', + 1 => 'bar', + 2 => false, + 3 => null, + 4 => array(), + 5 => new stdClass() + )) + ); + } + + public static function validArraysWithStringKeys() + { + return array( + array(array( + 'foo' => 'bar', + )), + array(array( + 'bar', + 'foo' => 'bar', + 'baz', + )), + ); + } + + public static function validArraysWithNumericKeys() + { + return array( + array(array( + 'foo', + 'bar' + )), + array(array( + '0' => 'foo', + '1' => 'bar', + )), + array(array( + 'bar', + '1' => 'bar', + 3 => 'baz' + )), + array(array( + -10000 => null, + '-10000' => null, + )), + array(array( + '-00000.00009' => 'foo' + )), + array(array( + 1 => 0 + )), + ); + } + + public static function validArraysWithIntegerKeys() + { + return array( + array(array( + 'foo', + 'bar,' + )), + array(array( + 100 => 'foo', + 200 => 'bar' + )), + array(array( + -100 => 'foo', + 0 => 'bar', + 100 => 'baz' + )), + array(array( + 'foo', + 'bar', + 1000 => 'baz' + )), + ); + } + + public static function invalidArrays() + { + return array( + array(new stdClass()), + array(15), + array('foo'), + array(new ArrayObject()), + ); + } + + public static function mergeArrays() + { + return array( + 'merge-integer-and-string keys' => array( + array( + 'foo', + 3 => 'bar', + 'baz' => 'baz' + ), + array( + 'baz', + ), + array( + 0 => 'foo', + 3 => 'bar', + 'baz' => 'baz', + 4 => 'baz' + ) + ), + 'merge-arrays-recursively' => array( + array( + 'foo' => array( + 'baz' + ) + ), + array( + 'foo' => array( + 'baz' + ) + ), + array( + 'foo' => array( + 0 => 'baz', + 1 => 'baz' + ) + ) + ), + 'replace-string-keys' => array( + array( + 'foo' => 'bar', + 'bar' => array() + ), + array( + 'foo' => 'baz', + 'bar' => 'bat' + ), + array( + 'foo' => 'baz', + 'bar' => 'bat' + ) + ), + ); + } + + public static function validIterators() + { + return array( + array(array( + 'foo' => 'bar', + ), array( + 'foo' => 'bar', + )), + array(new Config(array( + 'foo' => array( + 'bar' => array( + 'baz' => array( + 'baz' => 'bat', + ), + ), + ), + )), array( + 'foo' => array( + 'bar' => array( + 'baz' => array( + 'baz' => 'bat', + ), + ), + ), + )), + array(new ArrayObject(array( + 'foo' => array( + 'bar' => array( + 'baz' => array( + 'baz' => 'bat', + ), + ), + ), + )), array( + 'foo' => array( + 'bar' => array( + 'baz' => array( + 'baz' => 'bat', + ), + ), + ), + )), + ); + } + + public static function invalidIterators() + { + return array( + array(null), + array(true), + array(false), + array(0), + array(1), + array(0.0), + array(1.0), + array('string'), + array(new stdClass), + ); + } + + /** + * @dataProvider validArraysWithStringKeys + */ + public function testValidArraysWithStringKeys($test) + { + $this->assertTrue(ArrayTools::hasStringKeys($test)); + } + + /** + * @dataProvider validArraysWithIntegerKeys + */ + public function testValidArraysWithIntegerKeys($test) + { + $this->assertTrue(ArrayTools::hasIntegerKeys($test)); + } + + /** + * @dataProvider validArraysWithNumericKeys + */ + public function testValidArraysWithNumericKeys($test) + { + $this->assertTrue(ArrayTools::hasNumericKeys($test)); + } + + /** + * @dataProvider invalidArrays + */ + public function testInvalidArraysAlwaysReturnFalse($test) + { + $this->assertFalse(ArrayTools::hasStringKeys($test, False)); + $this->assertFalse(ArrayTools::hasIntegerKeys($test, False)); + $this->assertFalse(ArrayTools::hasNumericKeys($test, False)); + $this->assertFalse(ArrayTools::isList($test, False)); + $this->assertFalse(ArrayTools::isHashTable($test, False)); + + $this->assertFalse(ArrayTools::hasStringKeys($test, false)); + $this->assertFalse(ArrayTools::hasIntegerKeys($test, false)); + $this->assertFalse(ArrayTools::hasNumericKeys($test, false)); + $this->assertFalse(ArrayTools::isList($test, false)); + $this->assertFalse(ArrayTools::isHashTable($test, false)); + } + + /** + * @dataProvider validLists + */ + public function testLists($test) + { + $this->assertTrue(ArrayTools::isList($test)); + $this->assertTrue(ArrayTools::hasIntegerKeys($test)); + $this->assertTrue(ArrayTools::hasNumericKeys($test)); + $this->assertFalse(ArrayTools::hasStringKeys($test)); + $this->assertFalse(ArrayTools::isHashTable($test)); + } + + /** + * @dataProvider validHashTables + */ + public function testHashTables($test) + { + $this->assertTrue(ArrayTools::isHashTable($test)); + $this->assertFalse(ArrayTools::isList($test)); + } + + public function testEmptyArrayReturnsTrue() + { + $test = array(); + $this->assertTrue(ArrayTools::hasStringKeys($test, true)); + $this->assertTrue(ArrayTools::hasIntegerKeys($test, true)); + $this->assertTrue(ArrayTools::hasNumericKeys($test, true)); + $this->assertTrue(ArrayTools::isList($test, true)); + $this->assertTrue(ArrayTools::isHashTable($test, true)); + } + + public function testEmptyArrayReturnsFalse() + { + $test = array(); + $this->assertFalse(ArrayTools::hasStringKeys($test, false)); + $this->assertFalse(ArrayTools::hasIntegerKeys($test, false)); + $this->assertFalse(ArrayTools::hasNumericKeys($test, false)); + $this->assertFalse(ArrayTools::isList($test, false)); + $this->assertFalse(ArrayTools::isHashTable($test, false)); + } + + /** + * @dataProvider mergeArrays + */ + public function testMerge($a, $b, $expected) + { + $this->assertEquals($expected, ArrayTools::merge($a, $b)); + } + + /** + * @dataProvider validIterators + */ + public function testValidIteratorsReturnArrayRepresentation($test, $expected) + { + $result = ArrayTools::iteratorToArray($test); + $this->assertEquals($expected, $result); + } + + /** + * @dataProvider invalidIterators + */ + public function testInvalidIteratorsRaiseInvalidArgumentException($test) + { + $this->setExpectedException('Zend\Stdlib\Exception\InvalidArgumentException'); + $this->assertFalse(ArrayTools::iteratorToArray($test)); + } +} diff --git a/test/IsAssocArrayTest.php b/test/IsAssocArrayTest.php deleted file mode 100644 index d834dc241..000000000 --- a/test/IsAssocArrayTest.php +++ /dev/null @@ -1,79 +0,0 @@ - 'bar', - )), - array(array( - 'bar', - 'foo' => 'bar', - 'baz', - )), - ); - } - - public static function validAssocEmptyArrays() - { - $array = self::validAssocArrays(); - $array[] = array(array()); - return $array; - } - - public static function invalidAssocArrays() - { - return array( - array(null), - array(true), - array(false), - array(0), - array(1), - array(0.0), - array(1.0), - array('string'), - array(array(0, 1, 2)), - array(new stdClass), - ); - } - - /** - * @dataProvider validAssocArrays - */ - public function testValidAssocArraysReturnTrue($test) - { - $this->assertTrue(IsAssocArray::test($test)); - } - - /** - * @dataProvider validAssocEmptyArrays - */ - public function testValidAssocEmptyArraysReturnTrue($test) - { - $this->assertTrue(isAssocArray::test($test, true)); - } - - /** - * @dataProvider invalidAssocArrays - */ - public function testInvalidAssocArraysReturnFalse($test) - { - $this->assertFalse(IsAssocArray::test($test)); - } - - /** - * @dataProvider invalidAssocArrays - */ - public function testInvalidAssocEmptyArraysReturnFalse($test) - { - $this->assertFalse(IsAssocArray::test($test, true)); - } -} diff --git a/test/IteratorToArrayTest.php b/test/IteratorToArrayTest.php deleted file mode 100644 index 5f30b8fb4..000000000 --- a/test/IteratorToArrayTest.php +++ /dev/null @@ -1,90 +0,0 @@ - 'bar', - ), array( - 'foo' => 'bar', - )), - array(new Config(array( - 'foo' => array( - 'bar' => array( - 'baz' => array( - 'baz' => 'bat', - ), - ), - ), - )), array( - 'foo' => array( - 'bar' => array( - 'baz' => array( - 'baz' => 'bat', - ), - ), - ), - )), - array(new ArrayObject(array( - 'foo' => array( - 'bar' => array( - 'baz' => array( - 'baz' => 'bat', - ), - ), - ), - )), array( - 'foo' => array( - 'bar' => array( - 'baz' => array( - 'baz' => 'bat', - ), - ), - ), - )), - ); - } - - public static function invalidIterators() - { - return array( - array(null), - array(true), - array(false), - array(0), - array(1), - array(0.0), - array(1.0), - array('string'), - array(new stdClass), - ); - } - - /** - * @dataProvider validIterators - */ - public function testValidIteratorsReturnArrayRepresentation($test, $expected) - { - $result = IteratorToArray::convert($test); - $this->assertEquals($expected, $result); - } - - /** - * @dataProvider invalidIterators - */ - public function testInvalidIteratorsRaiseInvalidArgumentException($test) - { - $this->setExpectedException('Zend\Stdlib\Exception\InvalidArgumentException'); - $this->assertFalse(IteratorToArray::convert($test)); - } -} diff --git a/test/RecursiveArrayMergeTest.php b/test/RecursiveArrayMergeTest.php deleted file mode 100644 index 1dc6cf557..000000000 --- a/test/RecursiveArrayMergeTest.php +++ /dev/null @@ -1,71 +0,0 @@ - array( - array( - 'foo', - 3 => 'bar', - 'baz' => 'baz' - ), - array( - 'baz', - ), - array( - 0 => 'foo', - 3 => 'bar', - 'baz' => 'baz', - 4 => 'baz' - ) - ), - 'merge-arrays-recursively' => array( - array( - 'foo' => array( - 'baz' - ) - ), - array( - 'foo' => array( - 'baz' - ) - ), - array( - 'foo' => array( - 0 => 'baz', - 1 => 'baz' - ) - ) - ), - 'replace-string-keys' => array( - array( - 'foo' => 'bar', - 'bar' => array() - ), - array( - 'foo' => 'baz', - 'bar' => 'bat' - ), - array( - 'foo' => 'baz', - 'bar' => 'bat' - ) - ), - ); - } - - /** - * @dataProvider mergeArrays - */ - public function testMerge($a, $b, $expected) - { - $this->assertEquals($expected, RecursiveArrayMerge::merge($a, $b)); - } -} From 36a46ea9b13d5160554f755aea44c7964fc0ef3c Mon Sep 17 00:00:00 2001 From: Artur Bodera Date: Tue, 6 Mar 2012 16:54:06 +0100 Subject: [PATCH 07/11] Remove unneeded assignment, remove redundant RecursiveArrayMerge.php --- src/RecursiveArrayMerge.php | 64 ------------------------------------- 1 file changed, 64 deletions(-) delete mode 100644 src/RecursiveArrayMerge.php diff --git a/src/RecursiveArrayMerge.php b/src/RecursiveArrayMerge.php deleted file mode 100644 index 6d7a05d40..000000000 --- a/src/RecursiveArrayMerge.php +++ /dev/null @@ -1,64 +0,0 @@ - $value) { - if (array_key_exists($key, $a)) { - if (is_int($key)) { - $a[] = $value; - } elseif (is_array($value) && is_array($a[$key])) { - $a[$key] = self::merge($a[$key], $value); - } else { - $a[$key] = $value; - } - } else { - $a[$key] = $value; - } - } - - return $a; - } -} From 2b6512f688af11003ae8cd74859d79c265f0598e Mon Sep 17 00:00:00 2001 From: Thinkscape Date: Thu, 8 Mar 2012 17:29:22 +0100 Subject: [PATCH 08/11] Refactor Zend\Stdlib\ArrayTools to ArrayUtils, add missing headers. - ArrayTools will now be called ArrayUtils after an anonymous vote (http://framework.zend.com/wiki/display/ZFDEV2/POLL+-+Array+class+name). - Add missing Zend Framework headers to ArrayUtils class file and tests. --- src/{ArrayTools.php => ArrayUtils.php} | 25 +++++- ...{ArrayToolsTest.php => ArrayUtilsTest.php} | 90 +++++++++++-------- 2 files changed, 78 insertions(+), 37 deletions(-) rename src/{ArrayTools.php => ArrayUtils.php} (88%) rename test/{ArrayToolsTest.php => ArrayUtilsTest.php} (72%) diff --git a/src/ArrayTools.php b/src/ArrayUtils.php similarity index 88% rename from src/ArrayTools.php rename to src/ArrayUtils.php index f68ba2eff..e1940a10b 100644 --- a/src/ArrayTools.php +++ b/src/ArrayUtils.php @@ -1,15 +1,36 @@ assertTrue(ArrayTools::hasStringKeys($test)); + $this->assertTrue(ArrayUtils::hasStringKeys($test)); } /** @@ -260,7 +280,7 @@ public function testValidArraysWithStringKeys($test) */ public function testValidArraysWithIntegerKeys($test) { - $this->assertTrue(ArrayTools::hasIntegerKeys($test)); + $this->assertTrue(ArrayUtils::hasIntegerKeys($test)); } /** @@ -268,7 +288,7 @@ public function testValidArraysWithIntegerKeys($test) */ public function testValidArraysWithNumericKeys($test) { - $this->assertTrue(ArrayTools::hasNumericKeys($test)); + $this->assertTrue(ArrayUtils::hasNumericKeys($test)); } /** @@ -276,17 +296,17 @@ public function testValidArraysWithNumericKeys($test) */ public function testInvalidArraysAlwaysReturnFalse($test) { - $this->assertFalse(ArrayTools::hasStringKeys($test, False)); - $this->assertFalse(ArrayTools::hasIntegerKeys($test, False)); - $this->assertFalse(ArrayTools::hasNumericKeys($test, False)); - $this->assertFalse(ArrayTools::isList($test, False)); - $this->assertFalse(ArrayTools::isHashTable($test, False)); + $this->assertFalse(ArrayUtils::hasStringKeys($test, False)); + $this->assertFalse(ArrayUtils::hasIntegerKeys($test, False)); + $this->assertFalse(ArrayUtils::hasNumericKeys($test, False)); + $this->assertFalse(ArrayUtils::isList($test, False)); + $this->assertFalse(ArrayUtils::isHashTable($test, False)); - $this->assertFalse(ArrayTools::hasStringKeys($test, false)); - $this->assertFalse(ArrayTools::hasIntegerKeys($test, false)); - $this->assertFalse(ArrayTools::hasNumericKeys($test, false)); - $this->assertFalse(ArrayTools::isList($test, false)); - $this->assertFalse(ArrayTools::isHashTable($test, false)); + $this->assertFalse(ArrayUtils::hasStringKeys($test, false)); + $this->assertFalse(ArrayUtils::hasIntegerKeys($test, false)); + $this->assertFalse(ArrayUtils::hasNumericKeys($test, false)); + $this->assertFalse(ArrayUtils::isList($test, false)); + $this->assertFalse(ArrayUtils::isHashTable($test, false)); } /** @@ -294,11 +314,11 @@ public function testInvalidArraysAlwaysReturnFalse($test) */ public function testLists($test) { - $this->assertTrue(ArrayTools::isList($test)); - $this->assertTrue(ArrayTools::hasIntegerKeys($test)); - $this->assertTrue(ArrayTools::hasNumericKeys($test)); - $this->assertFalse(ArrayTools::hasStringKeys($test)); - $this->assertFalse(ArrayTools::isHashTable($test)); + $this->assertTrue(ArrayUtils::isList($test)); + $this->assertTrue(ArrayUtils::hasIntegerKeys($test)); + $this->assertTrue(ArrayUtils::hasNumericKeys($test)); + $this->assertFalse(ArrayUtils::hasStringKeys($test)); + $this->assertFalse(ArrayUtils::isHashTable($test)); } /** @@ -306,28 +326,28 @@ public function testLists($test) */ public function testHashTables($test) { - $this->assertTrue(ArrayTools::isHashTable($test)); - $this->assertFalse(ArrayTools::isList($test)); + $this->assertTrue(ArrayUtils::isHashTable($test)); + $this->assertFalse(ArrayUtils::isList($test)); } public function testEmptyArrayReturnsTrue() { $test = array(); - $this->assertTrue(ArrayTools::hasStringKeys($test, true)); - $this->assertTrue(ArrayTools::hasIntegerKeys($test, true)); - $this->assertTrue(ArrayTools::hasNumericKeys($test, true)); - $this->assertTrue(ArrayTools::isList($test, true)); - $this->assertTrue(ArrayTools::isHashTable($test, true)); + $this->assertTrue(ArrayUtils::hasStringKeys($test, true)); + $this->assertTrue(ArrayUtils::hasIntegerKeys($test, true)); + $this->assertTrue(ArrayUtils::hasNumericKeys($test, true)); + $this->assertTrue(ArrayUtils::isList($test, true)); + $this->assertTrue(ArrayUtils::isHashTable($test, true)); } public function testEmptyArrayReturnsFalse() { $test = array(); - $this->assertFalse(ArrayTools::hasStringKeys($test, false)); - $this->assertFalse(ArrayTools::hasIntegerKeys($test, false)); - $this->assertFalse(ArrayTools::hasNumericKeys($test, false)); - $this->assertFalse(ArrayTools::isList($test, false)); - $this->assertFalse(ArrayTools::isHashTable($test, false)); + $this->assertFalse(ArrayUtils::hasStringKeys($test, false)); + $this->assertFalse(ArrayUtils::hasIntegerKeys($test, false)); + $this->assertFalse(ArrayUtils::hasNumericKeys($test, false)); + $this->assertFalse(ArrayUtils::isList($test, false)); + $this->assertFalse(ArrayUtils::isHashTable($test, false)); } /** @@ -335,7 +355,7 @@ public function testEmptyArrayReturnsFalse() */ public function testMerge($a, $b, $expected) { - $this->assertEquals($expected, ArrayTools::merge($a, $b)); + $this->assertEquals($expected, ArrayUtils::merge($a, $b)); } /** @@ -343,7 +363,7 @@ public function testMerge($a, $b, $expected) */ public function testValidIteratorsReturnArrayRepresentation($test, $expected) { - $result = ArrayTools::iteratorToArray($test); + $result = ArrayUtils::iteratorToArray($test); $this->assertEquals($expected, $result); } @@ -353,6 +373,6 @@ public function testValidIteratorsReturnArrayRepresentation($test, $expected) public function testInvalidIteratorsRaiseInvalidArgumentException($test) { $this->setExpectedException('Zend\Stdlib\Exception\InvalidArgumentException'); - $this->assertFalse(ArrayTools::iteratorToArray($test)); + $this->assertFalse(ArrayUtils::iteratorToArray($test)); } } From 51413d02e0197dc63aa6bc71abb778cdd8c749dc Mon Sep 17 00:00:00 2001 From: Thinkscape Date: Thu, 8 Mar 2012 18:56:16 +0100 Subject: [PATCH 09/11] Minor cleanup --- src/ArrayUtils.php | 60 +++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/ArrayUtils.php b/src/ArrayUtils.php index e1940a10b..7c4f9f1ea 100644 --- a/src/ArrayUtils.php +++ b/src/ArrayUtils.php @@ -41,13 +41,13 @@ abstract class ArrayUtils */ public static function hasStringKeys($value, $allowEmpty = false) { - if (is_array($value)) { - if (count($value) == 0) { - return $allowEmpty; - } - return count(array_filter(array_keys($value), 'is_string')) > 0; + if (!is_array($value)) { + return false; + } elseif (!$value) { + return $allowEmpty; } - return false; + + return count(array_filter(array_keys($value), 'is_string')) > 0; } /** @@ -59,13 +59,13 @@ public static function hasStringKeys($value, $allowEmpty = false) */ public static function hasIntegerKeys($value, $allowEmpty = false) { - if (is_array($value)) { - if (count($value) == 0) { - return $allowEmpty; - } - return count(array_filter(array_keys($value), 'is_int')) > 0; + if (!is_array($value)) { + return false; + } elseif (!$value) { + return $allowEmpty; } - return false; + + return count(array_filter(array_keys($value), 'is_int')) > 0; } /** @@ -84,13 +84,13 @@ public static function hasIntegerKeys($value, $allowEmpty = false) */ public static function hasNumericKeys($value, $allowEmpty = false) { - if (is_array($value)) { - if (count($value) == 0) { - return $allowEmpty; - } - return count(array_filter(array_keys($value), 'is_numeric')) > 0; + if (!is_array($value)) { + return false; + } elseif (!$value) { + return $allowEmpty; } - return false; + + return count(array_filter(array_keys($value), 'is_numeric')) > 0; } /** @@ -113,13 +113,13 @@ public static function hasNumericKeys($value, $allowEmpty = false) */ public static function isList($value, $allowEmpty = false) { - if (is_array($value)) { - if (count($value) == 0) { - return $allowEmpty; - } - return (array_values($value) === $value); + if (!is_array($value)) { + return false; + } elseif (!$value) { + return $allowEmpty; } - return false; + + return (array_values($value) === $value); } /** @@ -150,13 +150,13 @@ public static function isList($value, $allowEmpty = false) */ public static function isHashTable($value, $allowEmpty = false) { - if (is_array($value)) { - if (count($value) == 0) { - return $allowEmpty; - } - return (array_values($value) !== $value); + if (!is_array($value)) { + return false; + } elseif (!$value) { + return $allowEmpty; } - return false; + + return (array_values($value) !== $value); } /** From 0353bd1ef72057992a9ce900e6ac4f32d987129c Mon Sep 17 00:00:00 2001 From: Thinkscape Date: Thu, 8 Mar 2012 19:02:58 +0100 Subject: [PATCH 10/11] Remove redundant "@namespace" --- src/ArrayUtils.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ArrayUtils.php b/src/ArrayUtils.php index 7c4f9f1ea..a91fa7db4 100644 --- a/src/ArrayUtils.php +++ b/src/ArrayUtils.php @@ -18,9 +18,6 @@ * @license http://framework.zend.com/license/new-bsd New BSD License */ -/** - * @namespace - */ namespace Zend\Stdlib; use \Traversable; From 9ae4cbc68de2d055833db8254c531712f316b493 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Fri, 9 Mar 2012 10:05:34 -0600 Subject: [PATCH 11/11] [ZF2-186] CS review of ArrayUtils - docblock fixes - imports should not use fully qualified names; imports always assume name is fully qualified - no need for elseif if the if statement has a return in it --- src/ArrayUtils.php | 86 ++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/src/ArrayUtils.php b/src/ArrayUtils.php index a91fa7db4..f3a9be7de 100644 --- a/src/ArrayUtils.php +++ b/src/ArrayUtils.php @@ -20,12 +20,17 @@ namespace Zend\Stdlib; -use \Traversable; +use Traversable; /** * Utility class for testing and manipulation of PHP arrays. * * Declared abstract, as we have no need for instantiation. + * + * @category Zend + * @package Zend_Stdlib + * @copyright Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com) + * @license http://framework.zend.com/license/new-bsd New BSD License */ abstract class ArrayUtils { @@ -40,7 +45,9 @@ public static function hasStringKeys($value, $allowEmpty = false) { if (!is_array($value)) { return false; - } elseif (!$value) { + } + + if (!$value) { return $allowEmpty; } @@ -58,7 +65,9 @@ public static function hasIntegerKeys($value, $allowEmpty = false) { if (!is_array($value)) { return false; - } elseif (!$value) { + } + + if (!$value) { return $allowEmpty; } @@ -69,11 +78,11 @@ public static function hasIntegerKeys($value, $allowEmpty = false) * Test whether an array contains one or more numeric keys. * * A numeric key can be one of the following: - * - an integer 1, - * - a string with a number '20' - * - a string with negative number: '-1000' - * - a float: 2.2120, -78.150999 - * - a strings with float: '4000.99999', '-10.10' + * - an integer 1, + * - a string with a number '20' + * - a string with negative number: '-1000' + * - a float: 2.2120, -78.150999 + * - a string with float: '4000.99999', '-10.10' * * @param mixed $value * @param bool $allowEmpty Should an empty array() return true @@ -83,7 +92,9 @@ public static function hasNumericKeys($value, $allowEmpty = false) { if (!is_array($value)) { return false; - } elseif (!$value) { + } + + if (!$value) { return $allowEmpty; } @@ -97,12 +108,14 @@ public static function hasNumericKeys($value, $allowEmpty = false) * starting at 0 and ending at count() - 1. * * For example: - * $list = array( 'a','b','c','d' ); - * $list = array( - * 0 => 'foo', - * 1 => 'bar', - * 2 => array( 'foo' => 'baz' ) - * ); + * + * $list = array( 'a','b','c','d' ); + * $list = array( + * 0 => 'foo', + * 1 => 'bar', + * 2 => array( 'foo' => 'baz' ), + * ); + * * * @param mixed $value * @param bool $allowEmpty Is an empty list a valid list? @@ -112,7 +125,9 @@ public static function isList($value, $allowEmpty = false) { if (!is_array($value)) { return false; - } elseif (!$value) { + } + + if (!$value) { return $allowEmpty; } @@ -123,23 +138,26 @@ public static function isList($value, $allowEmpty = false) * Test whether an array is a hash table. * * An array is a hash table if: - * 1. Contains one or more non-integer keys, or - * 2. Integer keys are non-continuous or misaligned (not starting with 0) + * + * 1. Contains one or more non-integer keys, or + * 2. Integer keys are non-continuous or misaligned (not starting with 0) * * For example: - * $hash = array( - * 'foo' => 15, - * 'bar' => false - * ); - * $hash = array( - * 1995 => 'Birth of PHP', - * 2009 => 'PHP 5.3.0', - * 2012 => 'PHP 5.4.0' - * ); - * $hash = array( - * 'formElement, - * 'options' => array( 'debug' => true ) - * ); + * + * $hash = array( + * 'foo' => 15, + * 'bar' => false, + * ); + * $hash = array( + * 1995 => 'Birth of PHP', + * 2009 => 'PHP 5.3.0', + * 2012 => 'PHP 5.4.0', + * ); + * $hash = array( + * 'formElement, + * 'options' => array( 'debug' => true ), + * ); + * * * @param mixed $value * @param bool $allowEmpty Is an empty array() a valid hash table? @@ -149,7 +167,9 @@ public static function isHashTable($value, $allowEmpty = false) { if (!is_array($value)) { return false; - } elseif (!$value) { + } + + if (!$value) { return $allowEmpty; } @@ -237,6 +257,4 @@ public static function merge(array $a, array $b) return $a; } - - }