I'm playing around with promises and I'm having trouble with an asynchronous recursive promise.
The scenario is an athlete starts running the 100m, I need to periodically check to see if they have finished and once they have finished, print their time.
Edit to clarify :
In the real world the athlete is running on a server. startRunning
involves making an ajax call to the server. checkIsFinished
also involves making an ajax call to the server. The code below is an attempt to imitate that. The times and distances in the code are hardcoded in an attempt to keep things as simple as possible. Apologies for not being clearer.
End edit
I'd like to be able to write the following
startRunning()
.then(checkIsFinished)
.then(printTime)
.catch(handleError)
where
var intervalID;
var startRunning = function () {
var athlete = {
timeTaken: 0,
distanceTravelled: 0
};
var updateAthlete = function () {
athlete.distanceTravelled += 25;
athlete.timeTaken += 2.5;
console.log("updated athlete", athlete)
}
intervalID = setInterval(updateAthlete, 2500);
return new Promise(function (resolve, reject) {
setTimeout(resolve.bind(null, athlete), 2000);
})
};
var checkIsFinished = function (athlete) {
return new Promise(function (resolve, reject) {
if (athlete.distanceTravelled >= 100) {
clearInterval(intervalID);
console.log("finished");
resolve(athlete);
} else {
console.log("not finished yet, check again in a bit");
setTimeout(checkIsFinished.bind(null, athlete), 1000);
}
});
};
var printTime = function (athlete) {
console.log('printing time', athlete.timeTaken);
};
var handleError = function (e) { console.log(e); };
I can see that the promise that is created the first time checkIsFinished
is never resolved. How can I ensure that that promise is resolved so that printTime
is called?
Instead of
resolve(athlete);
I could do
Promise.resolve(athlete).then(printTime);
But I'd like to avoid that if possible, I'd really like to be able to write
startRunning()
.then(checkIsFinished)
.then(printTime)
.catch(handleError)
The bug is that you are passing a function that returns a promise to
setTimeout
. That promise is lost into the ether. A band-aid fix might be to recurse on the executor function:But meh. I think this is a great example of why one should avoid the promise-constructor anti-pattern (because mixing promise code and non-promise code inevitably leads to bugs like this).
Best practices I follow to avoid such bugs:
After this, I find code easier to reason about and harder to bugger up, because everything follows the same pattern.
Applying this to your example got me here (I'm using es6 arrow functions for brevity. They work in Firefox and Chrome 45):
Note that
checkIsFinished
returns either athlete or a promise. This is fine here because.then
functions automatically promote return values from functions you pass in to promises. If you'll be callingcheckIsFinished
in other contexts, you might want to do the promotion yourself, usingreturn Promise.resolve(athlete);
instead ofreturn athlete;
.Edit in response to comments from Amit:
For a non-recursive answer, replace the entire
checkIsFinished
function with this helper:and then do this:
Using
setTimeout
/setInterval
is one of the scenrios that doesn't play well with promises, and causes you to use the frowned promise anti-pattern.Having said that, if you reconstruct your function make it a "wait for completion" type of function (and name it accordingly as well), you'd be able to solve your problem. The
waitForFinish
function is only called once, and returns a single promise (albeit a new one, on top of the original promise created instartRunning
). The handling of the recurrence throughsetTimeout
is done in an internal polling function, where proper try/catch is used to ensure exceptions are propagated to the promise.While all this code is functioning properly, a polling solution is never advised in an asynchronous environment and should be avoided if possible. In your case, since this sample mocks communication with a server, I'd consider using web sockets if possible.
Since your use of promises is pretty off the mark, it's a little hard to tell exactly what you're trying to do or what implementation would best fit, but here's a recommendation.
Promises are a one-shot state machine. As such, you return a promise and exactly one time in the future, the promise can be either rejected with a reason or resolved with a value. Given that design of promises, I think what makes sense would be something that can be used like this:
You could implement that with code like this:
Working demo: http://jsfiddle.net/jfriend00/fbmbrc8s/
FYI, my design preference would probably be to have a public athlete object and then methods on that object to start running, stop running, etc...
Here are some of the fundamental things you got wrong in a use of promises:
startRunning().then(checkIsFinished)
just doesn't make logical sense. For the first part of this to work,startRunning()
has to return a promise, and it has to resolve ore reject that promise when something useful happens. Your are just resolving it after two seconds which doesn't really seem to accomplish anything useful.Here's another approach that creates a public
Athlete()
object that has some methods and allows multiple people to be watching the progress: