[Suggestion] Code split for Array Utility

[gabbydgab]

Looking at some dependency graph for zend components, one way of requiring zend-stdlib is to use the array functions.

Some of these libraries are:

1. Zend\Config
2. Zend\Router
3. Zend\View
4. Zend\ConfigAggregator - (copy-pasted code because it ONLY needs the array utility)

Separating this may have a relevant value to packages that utilizes array functions such as the framework itself since the default configuration is in array.

Ported repository: https://github.com/php-refactoring/zend-array-utility

• Supports PHP 5.6
• Added QA checks like Zend coding standard and license header validation

Pros:

• Be able to upgrade the array utility classes, such as array sort, merge, find, etc.
• Will be able to stress test and benchmark all scenarios.
• We can have separate documentation on how to use this library.
• Zend Components and other packages that requires zend-stblib but not using the Array* classes will not be (that much) affected.

Cons:

• Zend components and other packages that requires zend-stblib and uses the Array* classes will be affected. Thus, it will require zendframework/zend-array-utility as dependecy for zend-stdlib to be BC compatible.

To do:

• Include the array utility package as zendframework/zend-array-utility as dependecy for zend-stdlib
• Integration testing for components requiring zend-stdlib and uses the Array* classes
• Dedicated documentation for array utilities

[gabbydgab]

Addendum:

This will also address to issues like:

• Minimize duplicated code on ArrayUtils #62
• Stdlib\Parameters - References Stdlib\ArrayObject constant whilst extending PhpArrayObject #43
• ArrayUtils::inArray should use strict in_array? #41

[Ocramius]

This seems like a good idea to me, since I generally require Zend\Stdlib just for some array processing and queue stuff.

I still want to drop PHP 5 support, especially considering that the array utils API didn't change over the past year, but I'll probably end up in a fighting pit with @weierophinney :-P

@gabbydgab there is still a massive issue with your repository though: you didn't import the original commits, which are in fact really important for tracing bugs and for attribution.

Also, a benchmark test suite us absolutely necessary from now on (see zendframework/zend-servicemanager#64).

[gabbydgab]

@Ocramius just copy-pasted it as of the moment, but if you can guide me on how to properly subsplit it from the upstream then I'll redo it.

[Ocramius]

@gabbydgab just cloning this repo and using git rm and git mv to change paths is sufficient ;-)

[gabbydgab]

@Ocramius updated the repository link and retains the copy-pasted codes here

[gabbydgab]

In addition to the QA toolkit, here's the output when I try check code quality using PHP Mess Detector

> phpmd src text codesize,unusedcode,naming,design,controversial
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayObject.php:22     The class ArrayObject has 20 public methods. Consider refactoring ArrayObject to keep number of public methods under 10.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayObject.php:22     The class ArrayObject has an overall complexity of 52 which is very high. The configured complexity threshold is 50.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayObject.php:409    Avoid variables with short names like $ar. Configured minimum length is 3.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayUtils.php:21      The class ArrayUtils has an overall complexity of 51 which is very high. The configured complexity threshold is 50.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayUtils.php:216     The method iteratorToArray() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayUtils.php:269     The method merge() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayUtils.php:269     Avoid variables with short names like $a. Configured minimum length is 3.
/var/www/projects/ZendFramework/ArrayUtility/src/ArrayUtils.php:269     Avoid variables with short names like $b. Configured minimum length is 3.
Script phpmd src text codesize,unusedcode,naming,design,controversial handling the phpmd event returned with error code 2

PS: Didn't change anything yet. It's all based on the current upstream. Also I didn't removed yet the Parameter* classes due to this hard dependency.

Thoughts, @Ocramius @weierophinney ?

[gabbydgab]

since athletic/athletic is no longer maintained, shall we use phpbench/phpbench?

[Ocramius]

Yep, phpbench FTW. As for the ccloc, ignore that for now.

…

On 7 Jan 2017 03:34, "Gab Amba" ***@***.***> wrote: since athletic/athletic <https://github.com/polyfractal/athletic> is no longer maintained, shall we use phpbench/phpbench <https://github.com/phpbench/phpbench>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#69 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakIGt7gUhzh1grQ26nhlqgBh4XiNDks5rPvnOgaJpZM4Lc-Ku> .

[gabbydgab]

Created concrete classes for IteratorToArray method to be backwards compatible for zend-stdlib:

1. SimpleIterator.php - not commonly used
2. RecursiveIterator.php - mostly used

It resolves ccloc for ArrayUtils::iteratorToArray() method.

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) {
        return SimpleIterator::toArray($iterator);
    }

    return RecursiveIterator::toArray($iterator);
}

TO DO:

• Provide benchmarks for the new Iterator classes
• Documentation for the new Iterator classes
• Additional test for ArrayUtils::iteratorToArray() that covers non-recursive interation

[Ocramius]

@Ocramius updated the repository link and retains the copy-pasted codes here

I don't see the history/past commits.

[Ocramius]

It resolves ccloc for ArrayUtils::iteratorToArray() method.

Please don't do that - the CCLOC is there due to past benchmarks. If you are doing a split, just do that and add issues for further improvements later.

[gabbydgab]

@Ocramius

The commits are in this repository: https://github.com/php-refactoring/zend-array-utility (develop branch)

Please don't do that - the CCLOC is there due to past benchmarks. If you are doing a split, just do that and add issues for further improvements later.

I will not touch that in the upstream (zend-stdlib); just want to demonstrate that the created classes (SimpleIterator and RecursiveIterator) can be used on the current logic.

Upstream code: ArrayUtils::iteratorToArray() method

    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 = [];
        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;
    }

Will be simplified to use the Iterator classes in the proposed Zend\ArrayUtils package for BC compatibility

    public static function iteratorToArray($iterator, $recursive = true)
    {
        if (! $recursive) {
            return SimpleIterator::toArray($iterator);
        }
        return RecursiveIterator::toArray($iterator);
    }

I'm thinking that, if applied, will set as deprecated method then suggest to use the appropriate array utily (Zend\ArrayUtils) package moving forward.

[gabbydgab]

Also, I've added some test (from the current ArrayUtilsTest) for RecursiveIteratorTest but there are scenarios that are not fully covered - like instance of Traversable and $array[$key] = $value; at the end of the foreach.

Can't find additional specifications on the tests and benchmark. Can you provide some guidance on this matter? @Ocramius

[gabbydgab]

@Ocramius @weierophinney

I've updated the repository and created a release tag (0.1.x) as extracted array utility functions from the current zend-stdlib library.

Added initial benchmarks for:

1. ArrayFilter
2. ArrayMerge
3. ArrayValidator
4. IteratorToArray Converter

PS: sample data for the benchmark is based on the current test data.

Optimization and extraction of functionalities will be next.

[gabbydgab]

Created issue for feature request: Checks if the provided array configuration is cache-able;

One of the best practices being taught is to use factories over closures - since closures are not cache-able.

practical usage:

private function cacheConfig(array $config, $cachedConfigFile)
{
    if (!ArrayUtils::isCachable($config)) {
          throw new InvalidArgumentException('Cannot cached config from %s; does not return array', $config);
    }

    // cache the config
}