From e4ce0ac48b09fbb01d9ad85a588a75d3f42b7474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 9 Oct 2019 17:36:32 +0200 Subject: [PATCH 1/2] Support native cloning of Graph, remove all createGraphClone*() methods --- src/Graph.php | 95 ++++++++------------------------------------- tests/GraphTest.php | 62 +++++++---------------------- 2 files changed, 29 insertions(+), 128 deletions(-) diff --git a/src/Graph.php b/src/Graph.php index f7e1f470..0085ec50 100644 --- a/src/Graph.php +++ b/src/Graph.php @@ -4,7 +4,6 @@ use Graphp\Graph\Attribute\AttributeAware; use Graphp\Graph\Attribute\AttributeBagReference; -use Graphp\Graph\Exception\BadMethodCallException; use Graphp\Graph\Exception\InvalidArgumentException; use Graphp\Graph\Exception\OutOfBoundsException; use Graphp\Graph\Exception\OverflowException; @@ -94,76 +93,6 @@ public function createVertexClone(Vertex $originalVertex) return $newVertex; } - /** - * create new clone/copy of this graph - copy all attributes and vertices, but do NOT copy edges - * - * using this method is faster than creating a new graph and calling createEdgeClone() yourself - * - * @return Graph - */ - public function createGraphCloneEdgeless() - { - $graph = new Graph(); - $graph->attributes = $this->attributes; - - foreach ($this->getVertices() as $originalVertex) { - $graph->createVertexClone($originalVertex); - } - - return $graph; - } - - /** - * create new clone/copy of this graph - copy all attributes and vertices. but only copy all given edges - * - * @param Edges|Edge[] $edges set or array of edges to be cloned - * @return Graph - * @uses Graph::createGraphCloneEdgeless() - * @uses Graph::createEdgeClone() for each edge to be cloned - */ - public function createGraphCloneEdges($edges) - { - $graph = $this->createGraphCloneEdgeless(); - foreach ($edges as $edge) { - $graph->createEdgeClone($edge); - } - - return $graph; - } - - /** - * create new clone/copy of this graph - copy all attributes, vertices and edges - * - * @return Graph - * @uses Graph::createGraphCloneEdges() to clone graph with current edges - */ - public function createGraphClone() - { - return $this->createGraphCloneEdges($this->edges); - } - - /** - * create a new clone/copy of this graph - copy all attributes and given vertices and its edges - * - * @param Vertices $vertices set of vertices to keep - * @return Graph - * @uses Graph::createGraphClone() to create a complete clone - * @uses Vertex::destroy() to remove unneeded vertices again - */ - public function createGraphCloneVertices($vertices) - { - $verticesKeep = Vertices::factory($vertices); - - $graph = $this->createGraphClone(); - foreach ($graph->getVertices()->getMap() as $vid => $vertex) { - if (!$verticesKeep->hasVertexId($vid)) { - $vertex->destroy(); - } - } - - return $graph; - } - /** * Creates a new undirected (bidirectional) edge between the given two vertices. * @@ -461,16 +390,24 @@ private function getEdgeCloneInternal(Edge $edge, Vertex $startVertex, Vertex $t } /** - * do NOT allow cloning of objects (MUST NOT be called!) - * - * @throws BadMethodCallException - * @see Graph::createGraphClone() instead + * create new clone/copy of this graph - copy all attributes, vertices and edges */ - private function __clone() + public function __clone() { - // @codeCoverageIgnoreStart - throw new BadMethodCallException(); - // @codeCoverageIgnoreEnd + $vertices = $this->verticesStorage; + $this->verticesStorage = array(); + $this->vertices = VerticesMap::factoryArrayReference($this->verticesStorage); + + $edges = $this->edgesStorage; + $this->edgesStorage = array(); + $this->edges = Edges::factoryArrayReference($this->edgesStorage); + + foreach ($vertices as $vertex) { + $this->createVertexClone($vertex); + } + foreach ($edges as $edge) { + $this->createEdgeClone($edge); + } } public function getAttribute($name, $default = null) diff --git a/tests/GraphTest.php b/tests/GraphTest.php index f62da19d..6461241c 100644 --- a/tests/GraphTest.php +++ b/tests/GraphTest.php @@ -39,13 +39,6 @@ public function testInvalidVertexClone() $graph->createVertexClone($vertex); } - public function testGraphCloneEmpty() - { - $graph = new Graph(); - $newgraph = $graph->createGraphClone(); - $this->assertGraphEquals($graph, $newgraph); - } - /** * @expectedException OutOfBoundsException */ @@ -55,38 +48,23 @@ public function testGetVertexNonexistant() $graph->getVertex('non-existant'); } - public function testGraphClone() + public function testGraphCloneNative() { $graph = new Graph(); - $graph->setAttribute('title', 'graph'); - - $vertex = $graph->createVertex(123); - $vertex->setAttribute('balanace', 10); - $vertex->setAttribute('group', 4); + $graph->setAttribute('color', 'grey'); + $v = $graph->createVertex(123)->setAttribute('color', 'blue'); + $graph->createEdgeDirected($v, $v)->setAttribute('color', 'red'); - $newgraph = $graph->createGraphClone(); + $newgraph = clone $graph; $this->assertGraphEquals($graph, $newgraph); - $graphClonedTwice = $newgraph->createGraphClone(); + $graphClonedTwice = clone $newgraph; $this->assertGraphEquals($graph, $graphClonedTwice); - } - - public function testGraphCloneEdgeless() - { - $graph = new Graph(); - $graph->createEdgeDirected($graph->createVertex(1), $graph->createVertex(2)); - $graph->createEdgeUndirected($graph->createVertex(3), $graph->getVertex(2)); - - $graphEdgeless = $graph->createGraphCloneEdgeless(); - - $graphExpected = new Graph(); - $graphExpected->createVertex(1); - $graphExpected->createVertex(2); - $graphExpected->createVertex(3); - $this->assertGraphEquals($graphExpected, $graphEdgeless); + $this->assertNotSame($graph->getEdges(), $newgraph->getEdges()); + $this->assertNotSame($graph->getVertices(), $newgraph->getVertices()); } /** @@ -400,7 +378,7 @@ public function testGetEdgesClones() $e1 = $graph->createEdgeDirected($v1, $v2); $e2 = $graph->createEdgeDirected($v2, $v1); - $graphClone = $graph->createGraphClone(); + $graphClone = clone $graph; $this->assertEdgeEquals($e1, $graphClone->getEdgeClone($e1)); $this->assertEdgeEquals($e2, $graphClone->getEdgeCloneInverted($e1)); @@ -432,30 +410,16 @@ public function testEdgesFailEdgeless() $v1 = $graph->createVertex(1); $v2 = $graph->createVertex(2); $e1 = $graph->createEdgeDirected($v1, $v2); - $graph->createEdgeDirected($v1, $v2); - $graphCloneEdgeless = $graph->createGraphCloneEdgeless(); + $graphCloneEdgeless = clone $graph; + foreach ($graphCloneEdgeless->getEdges() as $edge) { + $edge->destroy(); + } // nothing to return $graphCloneEdgeless->getEdgeClone($e1); } - public function testCreateGraphCloneVertices() - { - // 1 -- 2 -- 3 - $graph = new Graph(); - $v1 = $graph->createVertex(1); - $v2 = $graph->createVertex(2); - $v3 = $graph->createVertex(3); - $graph->createEdgeDirected($v1, $v2); - $graph->createEdgeDirected($v2, $v3); - - $graphClone = $graph->createGraphCloneVertices(array(1 => $v1, 2 => $v2)); - - $this->assertEquals(2, count($graphClone->getVertices())); - $this->assertEquals(1, count($graphClone->getEdges())); - } - protected function createAttributeAware() { return new Graph(); From 0966357e07021069fe5ffa88a51d09fb7340e858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 9 Oct 2019 20:05:20 +0200 Subject: [PATCH 2/2] Remove all methods related to cloning and cloned objects --- src/Graph.php | 166 +++++++------------------------------------- tests/GraphTest.php | 165 ++++++++++++++----------------------------- 2 files changed, 77 insertions(+), 254 deletions(-) diff --git a/src/Graph.php b/src/Graph.php index 0085ec50..5bb68258 100644 --- a/src/Graph.php +++ b/src/Graph.php @@ -7,8 +7,6 @@ use Graphp\Graph\Exception\InvalidArgumentException; use Graphp\Graph\Exception\OutOfBoundsException; use Graphp\Graph\Exception\OverflowException; -use Graphp\Graph\Exception\RuntimeException; -use Graphp\Graph\Exception\UnderflowException; use Graphp\Graph\Set\DualAggregate; use Graphp\Graph\Set\Edges; use Graphp\Graph\Set\Vertices; @@ -73,26 +71,6 @@ public function createVertex($id = NULL, $returnDuplicate = false) return new Vertex($this, $id); } - /** - * create a new Vertex in this Graph from the given input Vertex of another graph - * - * @param Vertex $originalVertex - * @return Vertex new vertex in this graph - * @throws RuntimeException if vertex with this ID already exists - */ - public function createVertexClone(Vertex $originalVertex) - { - $id = $originalVertex->getId(); - if ($this->vertices->hasVertexId($id)) { - throw new RuntimeException('Id of cloned vertex already exists'); - } - - $newVertex = new Vertex($this, $id); - $newVertex->getAttributeBag()->setAttributes($originalVertex->getAttributeBag()->getAttributes()); - - return $newVertex; - } - /** * Creates a new undirected (bidirectional) edge between the given two vertices. * @@ -127,69 +105,6 @@ public function createEdgeDirected(Vertex $source, Vertex $target) return new EdgeDirected($source, $target); } - /** - * create new clone of the given edge between adjacent vertices - * - * @param Edge $originalEdge original edge (not neccessarily from this graph) - * @return Edge new edge in this graph - * @uses Graph::createEdgeCloneInternal() - */ - public function createEdgeClone(Edge $originalEdge) - { - return $this->createEdgeCloneInternal($originalEdge, 0, 1); - } - - /** - * create new clone of the given edge inverted (in opposite direction) between adjacent vertices - * - * @param Edge $originalEdge original edge (not neccessarily from this graph) - * @return Edge new edge in this graph - * @uses Graph::createEdgeCloneInternal() - */ - public function createEdgeCloneInverted(Edge $originalEdge) - { - return $this->createEdgeCloneInternal($originalEdge, 1, 0); - } - - /** - * create new clone of the given edge between adjacent vertices - * - * @param Edge $originalEdge original edge from old graph - * @param int $ia index of start vertex - * @param int $ib index of end vertex - * @return Edge new edge in this graph - * @uses Edge::getVertices() - * @uses Graph::getVertex() - * @uses Vertex::createEdgeUndirected() to create a new undirected edge if given edge was undrected - * @uses Vertex::createEdgeDirected() to create a new directed edge if given edge was directed - * @uses Edge::getWeight() - * @uses Edge::setWeight() - * @uses Edge::getFlow() - * @uses Edge::setFlow() - * @uses Edge::getCapacity() - * @uses Edge::setCapacity() - */ - private function createEdgeCloneInternal(Edge $originalEdge, $ia, $ib) - { - $ends = $originalEdge->getVertices()->getIds(); - - // get start vertex from old start vertex id - $a = $this->getVertex($ends[$ia]); - // get target vertex from old target vertex id - $b = $this->getVertex($ends[$ib]); - - if ($originalEdge instanceof EdgeDirected) { - $newEdge = $this->createEdgeDirected($a, $b); - } else { - // create new edge between new a and b - $newEdge = $this->createEdgeUndirected($a, $b); - } - - $newEdge->getAttributeBag()->setAttributes($originalEdge->getAttributeBag()->getAttributes()); - - return $newEdge; - } - /** * create the given number of vertices or given array of Vertex IDs * @@ -337,58 +252,6 @@ public function removeVertex(Vertex $vertex) } } - /** - * Extracts edge from this graph - * - * @param Edge $edge - * @return Edge - * @throws UnderflowException if no edge was found - * @throws OverflowException if multiple edges match - */ - public function getEdgeClone(Edge $edge) - { - // Extract endpoints from edge - $vertices = $edge->getVertices()->getVector(); - - return $this->getEdgeCloneInternal($edge, $vertices[0], $vertices[1]); - } - - /** - * Extracts inverted edge from this graph - * - * @param Edge $edge - * @return Edge - * @throws UnderflowException if no edge was found - * @throws OverflowException if multiple edges match - */ - public function getEdgeCloneInverted(Edge $edge) - { - // Extract endpoints from edge - $vertices = $edge->getVertices()->getVector(); - - return $this->getEdgeCloneInternal($edge, $vertices[1], $vertices[0]); - } - - private function getEdgeCloneInternal(Edge $edge, Vertex $startVertex, Vertex $targetVertex) - { - // Get original vertices from resultgraph - $residualGraphEdgeStartVertex = $this->getVertex($startVertex->getId()); - $residualGraphEdgeTargetVertex = $this->getVertex($targetVertex->getId()); - - // Now get the edge - $residualEdgeArray = $residualGraphEdgeStartVertex->getEdgesTo($residualGraphEdgeTargetVertex); - $residualEdgeArray = Edges::factory($residualEdgeArray)->getVector(); - - // Check for parallel edges - if (!$residualEdgeArray) { - throw new UnderflowException('No original edges for given cloned edge found'); - } elseif (count($residualEdgeArray) !== 1) { - throw new OverflowException('More than one cloned edge? Parallel edges (multigraph) not supported'); - } - - return $residualEdgeArray[0]; - } - /** * create new clone/copy of this graph - copy all attributes, vertices and edges */ @@ -402,11 +265,32 @@ public function __clone() $this->edgesStorage = array(); $this->edges = Edges::factoryArrayReference($this->edgesStorage); - foreach ($vertices as $vertex) { - $this->createVertexClone($vertex); + $map = array(); + foreach ($vertices as $originalVertex) { + assert($originalVertex instanceof Vertex); + + $vertex = new Vertex($this, $originalVertex->getId()); + $vertex->getAttributeBag()->setAttributes($originalVertex->getAttributeBag()->getAttributes()); + + // create map with old vertex hash to new vertex object + $map[spl_object_hash($originalVertex)] = $vertex; } - foreach ($edges as $edge) { - $this->createEdgeClone($edge); + + foreach ($edges as $originalEdge) { + assert($originalEdge instanceof Edge); + + // use map to match old vertex hashes to new vertex objects + $vertices = $originalEdge->getVertices()->getVector(); + $v1 = $map[spl_object_hash($vertices[0])]; + $v2 = $map[spl_object_hash($vertices[1])]; + + // recreate edge and assign attributes + if ($originalEdge instanceof EdgeUndirected) { + $edge = $this->createEdgeUndirected($v1, $v2); + } else { + $edge = $this->createEdgeDirected($v1, $v2); + } + $edge->getAttributeBag()->setAttributes($originalEdge->getAttributeBag()->getAttributes()); } } diff --git a/tests/GraphTest.php b/tests/GraphTest.php index 6461241c..f752b54f 100644 --- a/tests/GraphTest.php +++ b/tests/GraphTest.php @@ -9,36 +9,6 @@ class GraphTest extends AbstractAttributeAwareTest { - public function setup() - { - $this->graph = new Graph(); - } - - public function testVertexClone() - { - $graph = new Graph(); - $vertex = $graph->createVertex(123); - $vertex->setAttribute('balance', 29); - $vertex->setAttribute('group', 4); - - $newgraph = new Graph(); - $newvertex = $newgraph->createVertexClone($vertex); - - $this->assertVertexEquals($vertex, $newvertex); - } - - /** - * test to make sure vertex can not be cloned into same graph (due to duplicate id) - * - * @expectedException RuntimeException - */ - public function testInvalidVertexClone() - { - $graph = new Graph(); - $vertex = $graph->createVertex(123); - $graph->createVertexClone($vertex); - } - /** * @expectedException OutOfBoundsException */ @@ -48,25 +18,6 @@ public function testGetVertexNonexistant() $graph->getVertex('non-existant'); } - public function testGraphCloneNative() - { - $graph = new Graph(); - $graph->setAttribute('color', 'grey'); - $v = $graph->createVertex(123)->setAttribute('color', 'blue'); - $graph->createEdgeDirected($v, $v)->setAttribute('color', 'red'); - - $newgraph = clone $graph; - - $this->assertGraphEquals($graph, $newgraph); - - $graphClonedTwice = clone $newgraph; - - $this->assertGraphEquals($graph, $graphClonedTwice); - - $this->assertNotSame($graph->getEdges(), $newgraph->getEdges()); - $this->assertNotSame($graph->getVertices(), $newgraph->getVertices()); - } - /** * check to make sure we can actually create vertices with automatic IDs */ @@ -282,38 +233,6 @@ public function testCreateVerticesContainsInvalid() $graph->createVertices(array(1, 2, array(), 3)); } - public function testCloneInvertedUndirectedIsAlmostOriginal() - { - // 1 -- 2 - $graph = new Graph(); - $v1 = $graph->createVertex(1); - $v2 = $graph->createVertex(2); - $edge = $graph->createEdgeUndirected($v1, $v2); - - $edgeInverted = $graph->createEdgeCloneInverted($edge); - - $this->assertInstanceOf(get_class($edge), $edgeInverted); - - $this->assertEquals($edge->getVertices()->getVector(), array_reverse($edgeInverted->getVertices()->getVector())); - } - - public function testCloneDoubleInvertedDirectedEdgeIsOriginal() - { - // 1 -> 2 - $graph = new Graph(); - $v1 = $graph->createVertex(1); - $v2 = $graph->createVertex(2); - $edge = $graph->createEdgeDirected($v1, $v2); - - $edgeInverted = $graph->createEdgeCloneInverted($edge); - - $this->assertInstanceOf(get_class($edge), $edgeInverted); - - $edgeInvertedAgain = $graph->createEdgeCloneInverted($edgeInverted); - - $this->assertEdgeEquals($edge, $edgeInvertedAgain); - } - public function testRemoveEdge() { // 1 -- 2 @@ -369,55 +288,75 @@ public function testRemoveVertexInvalid() $vertex->destroy(); } - public function testGetEdgesClones() + public function testGraphCloneEmptyGraph() { - // 1 -> 2 -> 1 $graph = new Graph(); - $v1 = $graph->createVertex(1); - $v2 = $graph->createVertex(2); - $e1 = $graph->createEdgeDirected($v1, $v2); - $e2 = $graph->createEdgeDirected($v2, $v1); - $graphClone = clone $graph; + $newgraph = clone $graph; - $this->assertEdgeEquals($e1, $graphClone->getEdgeClone($e1)); - $this->assertEdgeEquals($e2, $graphClone->getEdgeCloneInverted($e1)); + $this->assertCount(0, $newgraph->getVertices()); + $this->assertCount(0, $newgraph->getEdges()); + $this->assertGraphEquals($graph, $newgraph); + $this->assertNotSame($graph, $newgraph); } - /** - * @expectedException OverflowException - */ - public function testEdgesFailParallel() + public function testGraphCloneMixedEdges() { - // 1 -> 2, 1 -> 2 + // 1 -> 2 -- 3 $graph = new Graph(); - $v1 = $graph->createVertex(1); - $v2 = $graph->createVertex(2); - $e1 = $graph->createEdgeDirected($v1, $v2); + $v1 = $graph->createVertex(); + $v2 = $graph->createVertex(); + $v3 = $graph->createVertex(); $graph->createEdgeDirected($v1, $v2); + $graph->createEdgeUndirected($v2, $v3); + + $newgraph = clone $graph; - // which one to return? e1? e2? - $graph->getEdgeClone($e1); + $this->assertCount(3, $newgraph->getVertices()); + $this->assertCount(2, $newgraph->getEdges()); + $this->assertGraphEquals($graph, $newgraph); } - /** - * @expectedException UnderflowException - */ - public function testEdgesFailEdgeless() + public function testGraphCloneParallelEdges() { // 1 -> 2 + // | ^ + // \----/ $graph = new Graph(); - $v1 = $graph->createVertex(1); - $v2 = $graph->createVertex(2); - $e1 = $graph->createEdgeDirected($v1, $v2); + $v1 = $graph->createVertex(); + $v2 = $graph->createVertex(); + $graph->createEdgeDirected($v1, $v2); + $graph->createEdgeDirected($v1, $v2); - $graphCloneEdgeless = clone $graph; - foreach ($graphCloneEdgeless->getEdges() as $edge) { - $edge->destroy(); - } + $newgraph = clone $graph; + + $this->assertCount(2, $newgraph->getVertices()); + $this->assertCount(2, $newgraph->getEdges()); + $this->assertGraphEquals($graph, $newgraph); + } - // nothing to return - $graphCloneEdgeless->getEdgeClone($e1); + public function testGraphCloneLoopGraphWithAttributes() + { + // 1 -\ + // ^ | + // \--/ + $graph = new Graph(); + $graph->setAttribute('color', 'grey'); + $v = $graph->createVertex(123)->setAttribute('color', 'blue'); + $graph->createEdgeDirected($v, $v)->setAttribute('color', 'red'); + + $newgraph = clone $graph; + + $this->assertCount(1, $newgraph->getVertices()); + $this->assertCount(1, $newgraph->getEdges()); + $this->assertGraphEquals($graph, $newgraph); + + $graphClonedTwice = clone $newgraph; + + $this->assertGraphEquals($graph, $graphClonedTwice); + + $this->assertNotSame($graph->getEdges(), $newgraph->getEdges()); + $this->assertNotSame($graph->getVertices(), $newgraph->getVertices()); } protected function createAttributeAware()