Clone collection before foreach in DuplicateKey filter

[venimus]

fixes #44

[stephpy]

Thanks

[venimus]

Could you please create a release if you are satisfied

[stephpy]

https://github.com/stephpy/timeline/releases/tag/v1.0.4 Done ;)

[venimus]

:+1:

[Zeichen32]

Then you can use:

$clonedCollection = is_object($collection) ? clone $collection : $collection;

I don't know if it is a good or bad practice, but i think its a bit faster for large collections. :sweat_smile:

Some tests:

$collection = array(
    'foo' => 1,
    'bar' => 2
);

$clonedCollection = is_object($collection) ? clone $collection : $collection;

unset($clonedCollection['foo']);

var_dump($collection === $clonedCollection);
var_dump($collection);
var_dump($clonedCollection);

// bool(false)
// array(2) { ["foo"]=> int(1) ["bar"]=> int(2) }
// array(1) { ["bar"]=> int(2) }

$collection = (object) array(
    'foo' => 1,
    'bar' => 2
);

$clonedCollection = is_object($collection) ? clone $collection : $collection;

unset($clonedCollection->foo);

var_dump($collection === $clonedCollection);
var_dump($collection);
var_dump($clonedCollection);

// bool(false)
// object(stdClass)#1 (2) { ["foo"]=> int(1) ["bar"]=> int(2) }
// object(stdClass)#2 (1) { ["bar"]=> int(2) }

$collection = (object) array(
    'foo' => (object) array('baz' => 1),
    'bar' => (object) array('baz' => 2)
);

$clonedCollection = is_object($collection) ? clone $collection : $collection;

unset($clonedCollection->foo);

var_dump($collection === $clonedCollection);
var_dump($collection);
var_dump($clonedCollection);

// bool(false)
// object(stdClass)#3 (2) { ["foo"]=> object(stdClass)#1 (1) { ["baz"]=> int(1) } ["bar"]=> object(stdClass)#2 (1) { ["baz"]=> int(2) } }
// object(stdClass)#4 (1) { ["bar"]=> object(stdClass)#2 (1) { ["baz"]=> int(2) } }

[stephpy]

:+1:

[venimus]

But if it is an array, this solution don't "clone" it, which leads to the original issue.

[Zeichen32]

See my test no #1. The Array will be clone, because it's a scalar type.

[venimus]

When I tracked the issue my collection was array of objects. Since it contains references, foreach will not clone it automatically as if it was a simple array. I'm sure there are more beautiful implementations, but the speed improvement is questionable because this filter is after pagination. Meaning it will contain mostly small number of items. Furthermore foreach internally is quite optimized, while clone is something very similar as implementation. That's why I preferred a simple to read code. I actually think the whole function needs refactoring anyway, but this is for the project owner to decide.