Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

add async.cond, aliased as async.if #272

Closed
wants to merge 1 commit into from

7 participants

@michaelficarra

I just tried out this project and was amazed to find out that there was no conditional evaluation equivalent under the "control flow" section. I did my best to search through the issues, but didn't see any requests for it. So here's my version.

Its usage is pretty straightforward. You provide a test, which chooses which callback to call, along with a consequent and an alternate, which produce possibly-different values for the final argument, the continuation. Errors are passed along as the first argument, as seems to be the idiom. I used dynamic member access to set the if member since I wasn't sure if you support ES3 environments.

@EndangeredMassa

I don't have anything else to add. I would like this to exist in async as well.

@michaelficarra

@EndangeredMassa: It appears that nobody maintains this repo. It is overrun with submissions of good ideas and pull requests that should be trivially easy to merge, and yet nobody does anything about it. Maybe the maintainers are on vacation?

@SLM-Linus SLM-Linus commented on the diff
lib/async.js
@@ -90,6 +90,14 @@
async.setImmediate = setImmediate;
}
+ async.cond = function (test, consequent, alternate, callback) {
+ test(function (err, testResult) {
+ if (err) return callback(err);
+ testResult ? consequent(callback) : alternate(callback);
+ });
+ };
+ async['if'] = async.cond;

Why not async.if instead of async['if']?

ES3 support. See the last sentence of the PR description.

Ahh, cool, I've must have missed it, sorry :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@richthegeek

+1 for this idea. Although it's just syntactic sugar (as, it could be argued, all of Async is) it is something that I find myself wanting multiple times per day. Consider the two:

next = (cb) -> cb()
if condition is true
    next = my_function

next () ->
   # rest of my code
async.if (condition is true), my_function, () ->
  # rest of my code
@richthegeek

Although whilst writing this I noticed a possible ambiguity that needs to be handled.

If I pass only two functions to async.if, should it be assumed that the second is the alternate or the second is the callback? There are valid cases for both.

A possible solution is that the functions can be passed as null, so async.if(condition, my_function, null, callback)

@caolan
Owner

@richthegeek your examples are not equivalent as far as I can see, since async.if assumes an asynchronous test.
@michaelficarra I'm open to adding this, but could you please provide me with an example use-case?

@brianmaissy

I guess the use case would be something like:

async.if(HTTPRequestReturns200, makeAnotherHTTPRequest, writeToErrorLog, next);

as opposed to:

HTTPRequestReturns200(function(success){
  if(success){
    makeAnotherHTTPRequest(next);
  }else{
    writeToErrorLog(next);
  }
});
@CatTail

@caolan The benefit to have async.cond is we can save the code called after condition.
For example, as @brianmaissy said, original code is

HTTPRequestReturns200(function(success){
  if(success){
    makeAnotherHTTPRequest(next);
  }else{
    writeToErrorLog(next);
  }
});

or it can be more obviously

HTTPRequestReturns200(function(success){
  if(success){
    makeAnotherHTTPRequest();
    next();
  }else{
    writeToErrorLog();
    next();
  }
});

With async.cond, only one next required:

async.if(HTTPRequestReturns200, makeAnotherHTTPRequest, writeToErrorLog, next);
@michaelficarra

@caolan: Ping. Were those examples sufficient?

@caolan
Owner

I think this is clearer using an if statement inside a callback

@caolan caolan closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 20, 2013
  1. @michaelficarra
This page is out of date. Refresh to see the latest.
Showing with 31 additions and 0 deletions.
  1. +8 −0 lib/async.js
  2. +23 −0 test/test-async.js
View
8 lib/async.js
@@ -90,6 +90,14 @@
async.setImmediate = setImmediate;
}
+ async.cond = function (test, consequent, alternate, callback) {
+ test(function (err, testResult) {
+ if (err) return callback(err);
+ testResult ? consequent(callback) : alternate(callback);
+ });
+ };
+ async['if'] = async.cond;

Why not async.if instead of async['if']?

ES3 support. See the last sentence of the PR description.

Ahh, cool, I've must have missed it, sorry :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
async.each = function (arr, iterator, callback) {
callback = callback || function () {};
if (!arr.length) {
View
23 test/test-async.js
@@ -66,6 +66,29 @@ function getFunctionsObject(call_order) {
};
}
+exports['cond'] = function(test) {
+ var yesFn = function (cb) { return cb(null, true); },
+ noFn = function (cb) { return cb(null, false); },
+ errFn = function (cb) { return cb(new Error); },
+ oneFn = function (cb) { return cb(null, 'one'); },
+ twoFn = function (cb) { return cb(null, 'two'); };
+ test.expect(7);
+ test.equal(async.cond, async['if']);
+ async.cond(yesFn, oneFn, twoFn, function(err, result) {
+ test.equal(err, null);
+ test.equal(result, 'one');
+ async.cond(noFn, oneFn, twoFn, function(err, result) {
+ test.equal(err, null);
+ test.equal(result, 'two');
+ async.cond(errFn, oneFn, twoFn, function(err, result) {
+ test.ok(err);
+ test.equal(result, null);
+ test.done();
+ });
+ });
+ });
+};
+
exports['forever'] = function (test) {
test.expect(1);
var counter = 0;
Something went wrong with that request. Please try again.