Merge pull request #81 from clue/fix-mbf

Fix Moore-Bellman-Ford for unweighted edges
graphp · Dec 29, 2013 · aadcd1e · aadcd1e
2 parents fe95b81 + c235c39
commit aadcd1e
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,10 @@ you spot any mistakes.
 *   Fix: Stricter checks for invalid cycles, such as one with an invalid
     predecessor-map or no edges at all ([#87](https://github.com/clue/graph/issues/87)
 
+*   Fix: The `Algorithm\ShortestPath\MooreBellmanFord` now also works for unweighted
+    edges. This also fixes an issue where `Algorithm\DetectNegativeCycle` didn't work
+    for unweighted edges. ([#81](https://github.com/clue/graph/issues/81)
+
 *   Fix: Throwing an `UnexpectedValueException` if writing GraphViz Dot script
     to a temporary file fails and remove its debugging output
     ([#77](https://github.com/clue/graph/issues/77) and [#78](https://github.com/clue/graph/issues/78) @Metabor)

diff --git a/lib/Fhaculty/Graph/Algorithm/ShortestPath/MooreBellmanFord.php b/lib/Fhaculty/Graph/Algorithm/ShortestPath/MooreBellmanFord.php
@@ -43,7 +43,7 @@ private function bigStep(Edges $edges, array &$totalCostOfCheapestPathTo, array
                     // New possible costs of this path
                     $newCost = $totalCostOfCheapestPathTo[$fromVertex->getId()] + $edge->getWeight();
                     if (is_infinite($newCost)) {
-                        $newCost = $edge->getWeight();
+                        $newCost = $edge->getWeight() + 0;
                     }
 
                     // No path has been found yet

diff --git a/tests/Fhaculty/Graph/Algorithm/ShortestPath/BaseShortestPathTest.php b/tests/Fhaculty/Graph/Algorithm/ShortestPath/BaseShortestPathTest.php
@@ -116,6 +116,24 @@ public function testSeparateGraphsAreNotReachable()
         $alg->getEdgesTo($vg2);
     }
 
+    public function testGraphUnweighted()
+    {
+        // 1 -> 2
+        $graph = new Graph();
+        $v1 = $graph->createVertex(1);
+        $v2 = $graph->createVertex(2);
+        $e1 = $v1->createEdgeTo($v2);
+
+        $alg = $this->createAlg($v1);
+
+        $expectedWeight = $this->getExpectedWeight(array($e1));
+        $this->assertEquals($expectedWeight, $alg->getDistance($v2));
+        $this->assertEquals(array(2 => $expectedWeight), $alg->getDistanceMap());
+        $this->assertEquals(array($e1), $alg->getEdges()->getVector());
+        $this->assertEquals(array($e1), $alg->getEdgesTo($v2)->getVector());
+        $this->assertEquals(array(2), $alg->getVertices()->getIds());
+    }
+
     public function testGraphTwoComponents()
     {
         // 1 -[10]-> 2

diff --git a/tests/Fhaculty/Graph/Algorithm/ShortestPath/MooreBellmanFordTest.php b/tests/Fhaculty/Graph/Algorithm/ShortestPath/MooreBellmanFordTest.php
@@ -69,4 +69,28 @@ public function testLoopNegativeWeightIsCycle()
 
         $cycle = $alg->getCycleNegative();
     }
+
+    public function testNegativeComponentHasCycle()
+    {
+        // 1 -[1]-> 2     3 --[-1]--> 4
+        //                ^           |
+        //                \---[-2]----/
+        $graph = new Graph();
+        $v1 = $graph->createVertex(1);
+        $v2 = $graph->createVertex(2);
+        $v3 = $graph->createVertex(3);
+        $v4 = $graph->createVertex(4);
+        $e1 = $v1->createEdgeTo($v2)->setWeight(1);
+        $e2 = $v3->createEdgeTo($v4)->setWeight(-1);
+        $e3 = $v4->createEdgeTo($v3)->setWeight(-2);
+
+        // second component has a cycle
+        $alg = $this->createAlg($v3);
+        $cycle = $alg->getCycleNegative();
+
+        // first component does not have a cycle
+        $alg = $this->createAlg($v1);
+        $this->setExpectedException('UnderflowException');
+        $alg->getCycleNegative();
+    }
 }