move interval to options object that may be passed in as the 1st parameter

3 files changed, 76 insertions, 46 deletions

diff --git a/README.md b/README.md
index 947d30f..392f06d 100644
--- a/README.md
+++ b/README.md
@@ -1459,7 +1459,7 @@ new tasks much easier (and the code more readable).
 ---------------------------------------
 
 <a name="retry" />
-### retry([times = 5], task, [callback, interval = 0])
+### retry([opts = {times: 5, interval: 0}| 5], task, [callback])
 
 Attempts to get a successful response from `task` no more than `times` times before
 returning an error. If the task is successful, the `callback` will be passed the result
@@ -1468,14 +1468,14 @@ result (if any) of the final attempt.
 
 __Arguments__
 
-* `times` - An integer indicating how many times to attempt the `task` before giving up. Defaults to 5.
+* `opts` - Can be either an object with `times` and `interval` or a number. `times` is how many attempts should be made before giving up. `interval` is how long to wait inbetween attempts. Defaults to {times: 5, interval: 0}
+  * if a number is passed in it sets `times` only (with `interval` defaulting to 0).
 * `task(callback, results)` - A function which receives two arguments: (1) a `callback(err, result)`
   which must be called when finished, passing `err` (which can be `null`) and the `result` of
   the function's execution, and (2) a `results` object, containing the results of
   the previously executed functions (if nested inside another control flow).
 * `callback(err, results)` - An optional callback which is called when the
   task has succeeded, or after the final failed attempt. It receives the `err` and `result` arguments of the last attempt at completing the `task`.
-* `interval` - How long to wait in milliseconds before making another attempt. Defaults to 0 (aka don't wait).
 
 The [`retry`](#retry) function can be used as a stand-alone control flow by passing a
 callback, as shown below:
@@ -1486,6 +1486,12 @@ async.retry(3, apiMethod, function(err, result) {
 });
 ```
 
+```js
+async.retry({times: 3, interval: 200}, apiMethod, function(err, result) {
+    // do something with the result
+});
+```
+
 It can also be embeded within other control flow functions to retry individual methods
 that are not as reliable, like this:
 
diff --git a/lib/async.js b/lib/async.js
index 6df72d0..de50c76 100644
--- a/lib/async.js
+++ b/lib/async.js
@@ -602,48 +602,54 @@
     };
 
 
-    async.retry = function(/*[times,] task [, callback, interval]*/) {
+
+    async.retry = function(/*[times,] task [, callback]*/) {
         var DEFAULT_TIMES = 5;
         var DEFAULT_INTERVAL = 0;
 
         var attempts = [];
 
-        var times = DEFAULT_TIMES;
-        var interval = DEFAULT_INTERVAL;
-        var task;
-        var callback;
+        var opts = {
+          times: DEFAULT_TIMES,
+          interval: DEFAULT_INTERVAL
+        };
 
-        switch(arguments.length){
-          case 1: {
-            task = arguments[0];
-            break;
-          }
-          case 2 : {
-            task = arguments[0];
-            callback = arguments[1];
-            break;
-          }
-          case 3: {
-            times = arguments[0];
-            task = arguments[1];
-            callback = arguments[2];
-            break;
-          }
-          case 4: {
-            times = arguments[0];
-            task = arguments[1];
-            callback = arguments[2];
-            interval = arguments[3];
-            break;
-          }
-          default: {
-            throw new Error('Invalid arguments - must be either (task), (task, callback), (times, task, callback) or (times, task, callback, interval)');
+        function parseTimes(acc, t){
+          if(typeof t === 'number'){
+            acc.times = parseInt(t, 10) || DEFAULT_TIMES;
+          } else if(typeof t === 'object'){
+            acc.times = parseInt(t.times, 10) || DEFAULT_TIMES;
+            acc.interval = parseInt(t.interval, 10) || DEFAULT_INTERVAL;
+          } else {
+            throw new Error('Unsupported argument type for \'times\': ' + typeof(t));
           }
         }
 
-        // Make sure times and interval are numbers
-        times = parseInt(times, 10) || DEFAULT_TIMES;
-        interval = parseInt(interval, 10) || DEFAULT_INTERVAL;
+        switch(arguments.length){
+            case 1: {
+              opts.task = arguments[0];
+              break;
+            }
+            case 2 : {
+              if(typeof arguments[0] === 'number' || typeof arguments[0] === 'object'){
+                parseTimes(opts, arguments[0]);
+                opts.task = arguments[1];
+              } else {
+                opts.task = arguments[0];
+                opts.callback = arguments[1];
+              }
+              break;
+            }
+            case 3: {
+              parseTimes(opts, arguments[0]);
+              opts.task = arguments[1];
+              opts.callback = arguments[2];
+              break;
+            }
+            default: {
+              throw new Error('Invalid arguments - must be either (task), (task, callback), (times, task) or (times, task, callback)');
+            }
+          }
 
         function wrappedTask(wrappedCallback, wrappedResults) {
             function retryAttempt(task, finalAttempt) {
@@ -662,23 +668,23 @@
               };
             }
 
-            while (times) {
+            while (opts.times) {
 
-                var finalAttempt = !(times-=1);
-                attempts.push(retryAttempt(task, finalAttempt));
-                if(!finalAttempt && interval > 0){
-                  attempts.push(retryInterval(interval));
+                var finalAttempt = !(opts.times-=1);
+                attempts.push(retryAttempt(opts.task, finalAttempt));
+                if(!finalAttempt && opts.interval > 0){
+                  attempts.push(retryInterval(opts.interval));
                 }
-
             }
+
             async.series(attempts, function(done, data){
                 data = data[data.length - 1];
-                (wrappedCallback || callback)(data.err, data.result);
+                (wrappedCallback || opts.callback)(data.err, data.result);
             });
         }
 
         // If a callback is passed, run this as a controll flow
-        return callback ? wrappedTask() : wrappedTask;
+        return opts.callback ? wrappedTask() : wrappedTask;
     };
 
     async.waterfall = function (tasks, callback) {
diff --git a/test/test-async.js b/test/test-async.js
index b74e84f..5b6f803 100755
--- a/test/test-async.js
+++ b/test/test-async.js
@@ -739,7 +739,7 @@ exports['retry with interval when all attempts succeeds'] = function(test) {
         callback(error + callCount, erroredResult + callCount); // respond with indexed values
     }
     var start = new Date().getTime();
-    async.retry(times, fn, function(err, result){
+    async.retry({ times: times, interval: interval}, fn, function(err, result){
         var now = new Date().getTime();
         var duration = now - start;
         test.ok(duration > (interval * (times -1)),  'did not include interval');
@@ -747,7 +747,7 @@ exports['retry with interval when all attempts succeeds'] = function(test) {
         test.equal(err, error + times, "Incorrect error was returned");
         test.equal(result, erroredResult + times, "Incorrect result was returned");
         test.done();
-    }, interval);
+    });
 };
 
 exports['retry as an embedded task'] = function(test) {
@@ -771,6 +771,24 @@ exports['retry as an embedded task'] = function(test) {
     });
 };
 
+exports['retry as an embedded task with interval'] = function(test) {
+    var start = new Date().getTime();
+    var opts = {times: 5, interval: 100};
+
+    async.auto({
+        foo: function(callback){
+            callback(null, 'FOO');
+        },
+        retry: async.retry(opts, function(callback) {
+            callback('err');
+        })
+    }, function(){
+        var duration = new Date().getTime() - start;
+        test.ok(duration > ((opts.times -1) * opts.interval), "The duration should have been greater than ((times -1) * interval)");
+        test.done();
+    });
+};
+
 exports['waterfall'] = {
 
 'basic': function(test){