Open weierophinney opened 4 years ago
Addendum:
This will also address to issues like:
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-270969861
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 https://github.com/zendframework/zend-servicemanager/pull/64).
Originally posted by @Ocramius at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-270972950
@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.
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-270974019
@gabbydgab just cloning this repo and using git rm
and git mv
to change paths is sufficient ;-)
Originally posted by @Ocramius at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-270974245
@Ocramius updated the repository link and retains the copy-pasted codes here
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-270986705
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 ?
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271053410
since athletic/athletic is no longer maintained, shall we use phpbench/phpbench?
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271056947
Yep, phpbench FTW.
As for the ccloc, ignore that for now.
On 7 Jan 2017 03:34, "Gab Amba" notifications@github.com 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 https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271056947, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakIGt7gUhzh1grQ26nhlqgBh4XiNDks5rPvnOgaJpZM4Lc-Ku .
Originally posted by @Ocramius at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271057975
Created concrete classes for IteratorToArray method to be backwards compatible for zend-stdlib:
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:
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271107999
@Ocramius updated the repository link and retains the copy-pasted codes here
I don't see the history/past commits.
Originally posted by @Ocramius at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271119029
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.
Originally posted by @Ocramius at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271119068
@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.
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271120387
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
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271121258
@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:
PS: sample data for the benchmark is based on the current test data.
Optimization and extraction of functionalities will be next.
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271225909
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
}
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69#issuecomment-271467121
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:
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
Pros:
Cons:
To do:
Originally posted by @gabbydgab at https://github.com/zendframework/zend-stdlib/issues/69