Merge pull request #63 from clue/remove-graph-isempty

Remove Graph::isEmpty()
graphp · Aug 4, 2013 · 07498ef · 07498ef
2 parents 053bcfe + 58686ad
commit 07498ef
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,12 @@ As such, its static factory methods had to be renamed. Update your references if
 | `Cycle::factoryFromVertices()` | `Walk::factoryCycleFromVertices()` |
 | `Cycle::factoryFromEdges()` | `Walk::factoryCycleFromEdges()` |
 
+* BC break: Remove `Graph::isEmpty()` because it's not well-defined and might
+be confusing. Most literature suggests it should check for existing edges,
+whereas the old behavior was to check for existing vertices instead. Use either
+of the more transparent methods
+`Algorithm\Property\GraphProperty::isNull()` (old behavior) or (where applicable)
+`Algorithm\Property\GraphProperty::isEmpty()` ([#59](https://github.com/clue/graph/issues/59)).
 * BC break: Each of the above methods (`Walk::factoryCycleFromPredecessorMap()`,
 `Walk::factoryCycleFromVertices()`, `Walk::factoryCycleFromEdges()`) now
 actually makes sure the returned `Walk` instance is actually a valid Cycle,

diff --git a/lib/Fhaculty/Graph/Algorithm/Degree.php b/lib/Fhaculty/Graph/Algorithm/Degree.php
@@ -80,7 +80,7 @@ public function getDegreeMax()
     public function isRegular()
     {
         // an empty graph is considered regular
-        if ($this->graph->isEmpty()) {
+        if ($this->graph->getVertices()->isEmpty()) {
             return true;
         }
         try {

diff --git a/lib/Fhaculty/Graph/Algorithm/Property/GraphProperty.php b/lib/Fhaculty/Graph/Algorithm/Property/GraphProperty.php
@@ -14,13 +14,31 @@ class GraphProperty extends BaseGraph
     /**
      * checks whether this graph has no edges
      *
+     * Also known as empty Graph. An empty Graph contains no edges, but can
+     * possibly contain any number of isolated vertices.
+     *
      * @return boolean
      */
     public function isEdgeless()
     {
         return $this->graph->getEdges()->isEmpty();
     }
 
+    /**
+     * checks whether this graph is a null graph (no vertex - and thus no edges)
+     *
+     * Each Edge is incident to two Vertices, or in case of an loop Edge,
+     * incident to the same Vertex twice. As such an Edge can not exist when
+     * no Vertices exist. So if we check we have no Vertices, we can also be
+     * sure that no Edges exist either.
+     *
+     * @return boolean
+     */
+    public function isNull()
+    {
+        return $this->graph->getVertices()->isEmpty();
+    }
+
     /**
      * checks whether this graph is trivial (one vertex and no edges)
      *

diff --git a/lib/Fhaculty/Graph/Algorithm/Tree/BaseDirected.php b/lib/Fhaculty/Graph/Algorithm/Tree/BaseDirected.php
@@ -73,13 +73,13 @@ public function getVertexRoot()
      * checks if this is a tree
      *
      * @return boolean
-     * @uses Graph::isEmpty() to skip empty Graphs (an empty Graph is a valid tree)
+     * @uses Vertices::isEmpty() to skip empty Graphs (an empty Graph is a valid tree)
      * @uses self::getVertexRoot() to get root Vertex to start search from
      * @uses self::getVerticesSubtree() to count number of vertices connected to root
      */
     public function isTree()
     {
-        if ($this->graph->isEmpty()) {
+        if ($this->graph->getVertices()->isEmpty()) {
             return true;
         }
 

diff --git a/lib/Fhaculty/Graph/Algorithm/Tree/Undirected.php b/lib/Fhaculty/Graph/Algorithm/Tree/Undirected.php
@@ -49,13 +49,13 @@ class Undirected extends Tree
      * checks if this is a tree
      *
      * @return boolean
-     * @uses Graph::isEmpty() to skip empty Graphs (an empty Graph is a valid tree)
+     * @uses Vertices::isEmpty() to skip empty Graphs (an empty Graph is a valid tree)
      * @uses Graph::getVertexFirst() to get get get random "root" Vertex to start search from
      * @uses self::getVerticesSubtreeRecursive() to count number of vertices connected to root
      */
     public function isTree()
     {
-        if ($this->graph->isEmpty()) {
+        if ($this->graph->getVertices()->isEmpty()) {
             return true;
         }
 

diff --git a/lib/Fhaculty/Graph/Graph.php b/lib/Fhaculty/Graph/Graph.php
@@ -348,16 +348,6 @@ public function getVertexFirst()
         return $this->vertices->getVertexFirst();
     }
 
-    /**
-     * checks whether this graph is empty (no vertex - and thus no edges, aka null graph)
-     *
-     * @return boolean
-     */
-    public function isEmpty()
-    {
-        return $this->vertices->isEmpty();
-    }
-
     /**
      * adds a new Edge to the Graph (MUST NOT be called manually!)
      *

diff --git a/tests/Fhaculty/Graph/Algorithm/Property/PropertyGraphTest.php b/tests/Fhaculty/Graph/Algorithm/Property/PropertyGraphTest.php
@@ -11,6 +11,7 @@ public function testEmptyIsEdgeless()
 
         $alg = new GraphProperty($graph);
 
+        $this->assertTrue($alg->isNull());
         $this->assertTrue($alg->isEdgeless());
         $this->assertFalse($alg->isTrivial());
     }
@@ -22,6 +23,8 @@ public function testSingleVertexIsTrivial()
 
         $alg = new GraphProperty($graph);
 
+        $this->assertFalse($alg->isNull());
+        $this->assertTrue($alg->isEdgeless());
         $this->assertTrue($alg->isTrivial());
     }
 }