Skip to content

Instantly share code, notes, and snippets.

@JamesMGreene
Created November 24, 2012 15:47
Show Gist options
  • Save JamesMGreene/4140215 to your computer and use it in GitHub Desktop.
Save JamesMGreene/4140215 to your computer and use it in GitHub Desktop.
Using Q, is this the best way to pass the results of two sequenced promises to the subsequent `then`/`spread` clause?
'use strict';
var Q = require('q');
var cmd = new require('commander').Command();
var client = new require('myAwesomeApi').Client();
var getUsername = function(done) {
var username;
cmd.prompt('Username: ', function(name) {
if (!name) {
done(new Error('You must provide a username!'));
}
else {
done(null, username);
}
});
};
var getPassword = function(done) {
cmd.password('Password: ', '*', function(pass) {
if (!pass) {
done(new Error('You must provide a password!'));
}
else {
process.stdin.destroy();
done(null, pass);
}
});
};
Q.napply(getUsername, null, []).then(function(username) {
return Q.napply(getPassword, null, []).then(function(password) {
return Q.resolve([username, password]);
});
}).spread(function(username, password) {
return Q.napply(client.login, client, [username, password]);
})
@JamesMGreene
Copy link
Author

Is this the best way to pass the results of two sequenced promises to the subsequent then/spread clause?

I started by using Q.all (for getUsername and getPassword) but that didn't work correctly with the stdout messages for getUsername and getPassword (which makes sense given the way Q.all works).

@domenic
Copy link

domenic commented Nov 24, 2012

Essentially this is the problem of how to consume state that depends on previous steps. Generally this requires nesting, but as you've discovered, you can break out of the nesting by resolving with an array and passing that down the chain. It's a fine approach, if perhaps a bit overkill in this case.

A couple of minor tweaks to this approach:

  • Q.napply(getUsername, null, []) can just be Q.napply(getUsername). Similarly for getPassword. I prefer ncall when I don't have an array, but that doesn't matter.
  • Q.napply(client.login, client, [username, password]) can just be Q.ninvoke(client, "login", username, password).
  • No need to wrap return values from within promise handlers in Q.resolve.
  • Don't forget to cap your promise chains with .done()!

Thus if I were to take this approach I'd do:

Q.ncall(getUsername).then(function (username) {
  return Q.ncall(getPassword).then(function (password) {
    return [username, password];
  });
}).spread(function (username, password) {
  return Q.ninvoke(client, "login", username, password);
}).done();

However, I really like making these things a bit shorter using Q.bind. Then you can do the nesting without it feeling too noisy:

var getUsernameP = Q.nbind(getUsername);
var getPasswordP = Q.nbind(getPassword);
var loginP = Q.nbind(client.login, client);

getUsernameP().then(function (username) {
  return getPasswordP().then(function (password) {
    return loginP(username, password);
  });
}).done();

@JamesMGreene
Copy link
Author

@domenic: Thanks for the feedback. I knew some of that (e.g. use Q.ncall and drop unnecessary params) but didn't know plenty of the rest is news to me (e.g. I've never noticed the use of done as a finalizer!). Some things I'm just trying out to get used to the signatures, etc
, but other things I may disagree with you on stylistically. :)

@kriskowal: Anything to add?

@JamesMGreene
Copy link
Author

@domenic: The API Reference mentions an end function but not a done function. Are they aliases for each other? Is either deprecated, or more likely to stick around (e.g. required by Promises/A+)?

@domenic
Copy link

domenic commented Nov 26, 2012

API reference is pretty outdated :-/. end is deprecated; done is a superset of its functionality (roughly, .then(f, r, p).end() <-> .done(f, r, p)).

Also we just deprecated Q.nbind in 0.8.11 so those lines above become

var getUsernameP = Q.nfbind(getUsername);
var getPasswordP = Q.nfbind(getPassword);
var loginP = Q.nfbind(client.login.bind(client));

@domenic
Copy link

domenic commented Nov 26, 2012

See here for a full list of deprecations and alternatives.

@kriskowal
Copy link

@JamesMGreene I think @domenic covered it all, except of course that if you happen to be in Firefox, you could use Q.async.

Q.async(function () {
    let username = yield getUsernameP();
    let password = yield getPasswordP();
    Q.return(loginP(username, password));
})().done();

Or in ES+generators:

Q.async(function *() {
    let username = yield getUsernameP();
    let password = yield getPasswordP();
    return loginP(username, password);
})().done();

@JamesMGreene
Copy link
Author

@kriskowal: Thanks for the info on Firefox advantages but this particular set of code is all for Node.js!

@JamesMGreene
Copy link
Author

@domenic:
Thanks, I grabbed 0.8.11 and updated all of my nbind calls to nfbinds. However, I am curious about the done method: if I have a fail handler in my promise chain, does it need to come before the done handler? Previously, my promise chain ended as .then(...).fail(...).end(); but I get the impression that it now needs to end as .fail(...).done(...);. Correct?

I assume I could still have it structured as before but just replace end with done (so .then(...).fail(...).done(); but it sounds like that would actually be a slightly sub-optimal setup due to the extra then hiding inside of then that would be used superfluously... right?

@JamesMGreene
Copy link
Author

FWIW, using .fail(...).done(...); feels very unnatural to me as the done handler is receiving a resolved promise value from my previous then but the fail handler is in the middle of the two, creating a mentally diverted/disjoint flow. Recommendations to fix that?

Where do you normally put your fail handler? Or, do you instead use the done handler's rejection argument (2nd function)? Would the done handler's rejection function receive failures from any point in the promise chain like fail does? I'm guessing not.

@domenic
Copy link

domenic commented Nov 26, 2012

No, fail shouldn't go before done; in that case: it does change the semantics.

.then(f1, r1, p1).fail(f2).end() should be replaced with .then(f1, r1, p1).fail(f2).done() or .then(f1, r2, p1).done(undefined, f2). I often cap with an empty .done(); I sometimes use .done(f) or sometimes .done(f, r).

Also: if you're writing only for Node you can use .catch instead of .fail (and .finally instead of .fin).

@JamesMGreene
Copy link
Author

Cool, I'll go with catch (definitely prefer its obviously semantic name over fail) and an empty done. Thanks again!

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