loophp / phptree

An implementation of tree data structure
MIT License
93 stars 4 forks source link

NodeInterface::clone() should not use unserialize(serialize()). #6

Closed drupol closed 5 years ago

drupol commented 5 years ago

The current NodeInterface::clone() method uses unserialize() and serialize().

This is not the best solution to clone an object, ideally we should have used __clone().

I've tests many solutions and the only one that works is with unserialize(serialize()).

You can easily test your solution using this code:

<?php

declare(strict_types = 1);

use Graphp\GraphViz\GraphViz;

include './vendor/autoload.php';

$exporterGraph = new \drupol\phptree\Exporter\Graph();

$root = new \drupol\phptree\Node\ValueNode('root', 2);
$root[] = new \drupol\phptree\Node\ValueNode('foo');

$clone = $root->clone();

echo 'Root count: ' . iterator_count($root->all()) . "\n";
echo 'Second clone count: ' . iterator_count($clone->all()) . "\n";

var_dump($root === $root[0]->getParent());
var_dump(sha1(spl_object_hash($root)) . ' === ' . sha1(spl_object_hash($root[0]->getParent())));
(new GraphViz())->setFormat('svg')->display($exporterGraph->export($root));

var_dump($clone === $clone[0]->getParent());
var_dump(sha1(spl_object_hash($clone)) . ' === ' .sha1(spl_object_hash($clone[0]->getParent())));
(new GraphViz())->setFormat('svg')->display($exporterGraph->export($clone));
drupol commented 5 years ago

Fixed.