Refactor Collection a bit.

easychen · Sep 10, 2016 · 71d4849 · 71d4849
1 parent 7ec4e7c
commit 71d4849
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 55 deletions.
diff --git a/src/Collection/Collection.php b/src/Collection/Collection.php
@@ -13,7 +13,6 @@
 use Robo\Contract\CommandInterface;
 
 
-use Robo\Contract\ProgressIndicatorAwareInterface;
 use Robo\Common\ProgressIndicatorAwareTrait;
 use Robo\Contract\InflectionInterface;
 
@@ -244,16 +243,6 @@ public function hasTask($name)
         return array_key_exists($name, $this->taskList);
     }
 
-    /**
-     * Test to see if the given name is an unnamed task, or
-     * something functionally equivalent.  Any numeric index
-     * is renumbered when added to the collection.
-     */
-    public static function isUnnamedTask($name)
-    {
-        return is_numeric($name);
-    }
-
     /**
      * Find an existing named task.
      *
@@ -300,7 +289,7 @@ protected function addCollectionElementToTaskList($name, Element $taskGroup)
     {
         // If a task name is not provided, then we'll let php pick
         // the array index.
-        if (static::isUnnamedTask($name)) {
+        if (Result::isUnnamed($name)) {
             $this->taskList[] = $taskGroup;
             return $this;
         }
@@ -397,19 +386,7 @@ public function progressIndicatorSteps()
     {
         $steps = 0;
         foreach ($this->taskList as $name => $taskGroup) {
-            foreach ($taskGroup->getTaskList() as $task) {
-                if ($task instanceof WrappedTaskInterface) {
-                    $task = $task->original();
-                }
-                // If the task is a ProgressIndicatorAwareInterface, then it
-                // will advance the progress indicator a number of times.
-                if ($task instanceof ProgressIndicatorAwareInterface) {
-                    $steps += $task->progressIndicatorSteps();
-                }
-                // We also advance the progress indicator once regardless
-                // of whether it is progress-indicator aware or not.
-                $steps++;
-            }
+            $steps += $taskGroup->progressIndicatorSteps();
         }
         return $steps;
     }
@@ -482,7 +459,7 @@ private function runWithoutCompletion()
      * Run every task in a list, but only up to the first failure.
      * Return the failing result, or success if all tasks run.
      */
-    private function runTaskList($name, array $taskList, $result)
+    private function runTaskList($name, array $taskList, Result $result)
     {
         try {
             foreach ($taskList as $taskName => $task) {
@@ -496,8 +473,8 @@ private function runTaskList($name, array $taskList, $result)
                 // We accumulate our results into a field so that tasks that
                 // have a reference to the collection may examine and modify
                 // the incremental results, if they wish.
-                $key = static::isUnnamedTask($taskName) ? $name : $taskName;
-                $result = $this->accumulateResults($key, $result, $taskResult);
+                $key = Result::isUnnamed($taskName) ? $name : $taskName;
+                $result->accumulate($key, $taskResult);
             }
         } catch (TaskExitException $exitException) {
             $this->fail();
@@ -578,33 +555,6 @@ protected function setParentCollectionForTask($task, $parentCollection)
         }
     }
 
-    /**
-     * Add the results from the most recent task to the accumulated
-     * results from all tasks that have run so far, merging data
-     * as necessary.
-     */
-    public function accumulateResults($key, Result $result, Result $taskResult)
-    {
-        // If the result is not set or is not a Result, then ignore it
-        if (isset($result) && ($result instanceof Result)) {
-            // If the task is unnamed, then all of its data elements
-            // just get merged in at the top-level of the final Result object.
-            if (static::isUnnamedTask($key)) {
-                $result->merge($taskResult);
-            } elseif (isset($result[$key])) {
-                // There can only be one task with a given name; however, if
-                // there are tasks added 'before' or 'after' the named task,
-                // then the results from these will be stored under the same
-                // name unless they are given a name of their own when added.
-                $current = $result[$key];
-                $result[$key] = $taskResult->merge($current);
-            } else {
-                $result[$key] = $taskResult;
-            }
-        }
-        return $result;
-    }
-
     /**
      * Run all of the tasks in a provided list, ignoring failures.
      * This is used to roll back or complete.

diff --git a/src/Collection/Element.php b/src/Collection/Element.php
@@ -3,6 +3,8 @@
 namespace Robo\Collection;
 
 use Robo\Contract\TaskInterface;
+use Robo\Contract\WrappedTaskInterface;
+use Robo\Contract\ProgressIndicatorAwareInterface;
 
 /**
  * One element in a collection.  Each element consists of a task
@@ -58,4 +60,23 @@ public function getTaskList()
     {
         return array_merge($this->getBefore(), [$this->getTask()], $this->getAfter());
     }
+
+    public function progressIndicatorSteps()
+    {
+        $steps = 0;
+        foreach ($this->getTaskList() as $task) {
+            if ($task instanceof WrappedTaskInterface) {
+                $task = $task->original();
+            }
+            // If the task is a ProgressIndicatorAwareInterface, then it
+            // will advance the progress indicator a number of times.
+            if ($task instanceof ProgressIndicatorAwareInterface) {
+                $steps += $task->progressIndicatorSteps();
+            }
+            // We also advance the progress indicator once regardless
+            // of whether it is progress-indicator aware or not.
+            $steps++;
+        }
+        return $steps;
+    }
 }
diff --git a/src/Result.php b/src/Result.php
@@ -87,6 +87,39 @@ public function getContext()
         ];
     }
 
+    /**
+     * Add the results from the most recent task to the accumulated
+     * results from all tasks that have run so far, merging data
+     * as necessary.
+     */
+    public function accumulate($key, Result $taskResult)
+    {
+        // If the task is unnamed, then all of its data elements
+        // just get merged in at the top-level of the final Result object.
+        if (static::isUnnamed($key)) {
+            $this->merge($taskResult);
+        } elseif (isset($this[$key])) {
+            // There can only be one task with a given name; however, if
+            // there are tasks added 'before' or 'after' the named task,
+            // then the results from these will be stored under the same
+            // name unless they are given a name of their own when added.
+            $current = $this[$key];
+            $this[$key] = $taskResult->merge($current);
+        } else {
+            $this[$key] = $taskResult;
+        }
+    }
+
+    /**
+     * We assume that named values (e.g. for associative array keys)
+     * are non-numeric; numeric keys are presumed to simply be the
+     * index of an array, and therefore insignificant.
+     */
+    public static function isUnnamed($key)
+    {
+        return is_numeric($key);
+    }
+
     /**
      * @return TaskInterface
      */