patternjavascriptMinor
Async Implementation
Viewed 0 times
asyncimplementationstackoverflow
Problem
I'm seeking code review comments for the following implementation.
jsFiddle
jsFiddle
/**
* You need to run a number of async tasks, but you're being rate limited.
* Run as many concurrent tasks as you can, queueing the rest for when the
* tasks are complete.
*/
/**
* @param concurrency: number of concurrent tasks that can be run
*/
var ConcurrencyQueue = function (concurrency) {
this.concurrency = concurrency;
this.queue = [];
this.count = 0;
};
/**
* add an asynchronous task to the queue. it will get executed immediately
* until the concurrent task count reaches this.concurrency. then, queue it until
* tasks are complete.
* @param task: function which takes a callback argument (done)
*/
ConcurrencyQueue.prototype.enqueue = function (task) {
var self = this;
self.count = self.count + 1;
var done = function () {
self.count = self.count - 1;
if (self.queue.length) {
self.queue.splice(0, 1)[0](done);
}
};
if (self.count > self.concurrency) {
self.queue.push(task);
}
if (self.count <= self.concurrency) {
task(done);
}
};
/**
* tasks passed to concurrency queue are async and call the callback "done"
* after they complete.
*/
function exampleTask (done) {
console.log('started');
setTimeout(function () {
console.log('completed');
done();
}, Math.floor(Math.random() * 2000));
};
var queue = new ConcurrencyQueue(2);
queue.enqueue(exampleTask);
queue.enqueue(exampleTask);
queue.enqueue(exampleTask);
queue.enqueue(exampleTask);
queue.enqueue(exampleTask);
queue.enqueue(exampleTask);
queue.enqueue(exampleTask);Solution
I have very little to say, maybe others will have more.
Why not
There's also a nitpick about
About the callbacks, you could move to the newer
jsFiddle
if (self.count > self.concurrency) {
self.queue.push(task);
}
if (self.count <= self.concurrency) {
task(done);
}Why not
if / else? At least it'll be clear it's one or the other.this.queue.shift() is cleaner than this.queue.splice(0, 1)[0].There's also a nitpick about
self.count = self.count + 1; where += 1 could be used instead, but it really depends on the coding style.About the callbacks, you could move to the newer
Promise which is part of ECMAScript 2015.ConcurrencyQueue.prototype.enqueue = function (task) {
this.count += 1;
if (this.count > this.concurrency) {
this.queue.push(task);
} else {
this.execute(task);
}
};
ConcurrencyQueue.prototype.execute = function(task) {
var p = new Promise(task);
/* `() => {` is a simpler notation for `function() {` in recent JS,
but `this` refers to the encompassing `this` */
var done = () => {
this.count -= 1;
if (this.queue.length) {
this.execute(this.queue.shift());
}
};
/* First argument is in case of success, second in case of errors */
p.then(done, done);
}jsFiddle
Code Snippets
if (self.count > self.concurrency) {
self.queue.push(task);
}
if (self.count <= self.concurrency) {
task(done);
}ConcurrencyQueue.prototype.enqueue = function (task) {
this.count += 1;
if (this.count > this.concurrency) {
this.queue.push(task);
} else {
this.execute(task);
}
};
ConcurrencyQueue.prototype.execute = function(task) {
var p = new Promise(task);
/* `() => {` is a simpler notation for `function() {` in recent JS,
but `this` refers to the encompassing `this` */
var done = () => {
this.count -= 1;
if (this.queue.length) {
this.execute(this.queue.shift());
}
};
/* First argument is in case of success, second in case of errors */
p.then(done, done);
}Context
StackExchange Code Review Q#129894, answer score: 2
Revisions (0)
No revisions yet.