Skip to main content
Code Review

Return to Answer

added 1727 characters in body
Source Link
Joseph
Joseph
  • 25.2k
  • 2
  • 25
  • 37
function Queue() {
  this.queue = []
}
Queue.prototype = {
  constructor: Queue,
  enqueue: function (fn, queueName) {
    this.queue.push({
      name: queueName || 'global',
      fn: fn || function (next) {
        next()
      }
    });
    return this
  },
  dequeue: function (queueName) {
    var allFns = (!queueName) ? this.queue : this.queue.filter(function (current) {
      return (current.name === queueName)
    });
    var poppedFn = allFns.pop();
    if (poppedFn) poppedFn.fn.call(this);
    return this
  },
  dequeueAll: function (queueName) {
    var instance = this;
    var queue = this.queue;
    var allFns = (!queueName) ? this.queue : this.queue.filter(function (current) {
      return (current.name === queueName)
    });
    (function recursive(index) {
      var currentItem = allFns[index];
      if (!currentItem) return;
      currentItem.fn.call(instance, function () {
        queue.splice(queue.indexOf(currentItem), 1);
        recursive(index)
      })
    }(0));
    return this
  }
};

var myQueue = new Queue();
myQueue.enqueue(function (next) {
  console.log('D1: first test1');
  next()
}, 'first').enqueue(function (next) {
  setTimeout(function () {
    console.log('D1: first test2');
    next()
  }, 2000)
}, 'first').enqueue(function (next) {
  console.log('D1: second test1');
  next()
}, 'second').enqueue(function (next) {
  console.log('D1: second test2');
  next()
}, 'second').dequeueAll();

function Queue() {
  this.queue = []
}
Queue.prototype = {
  constructor: Queue,
  enqueue: function (fn, queueName) {
    this.queue.push({
      name: queueName || 'global',
      fn: fn || function (next) {
        next()
      }
    });
    return this
  },
  dequeue: function (queueName) {
    var allFns = (!queueName) ? this.queue : this.queue.filter(function (current) {
      return (current.name === queueName)
    });
    var poppedFn = allFns.pop();
    if (poppedFn) poppedFn.fn.call(this);
    return this
  },
  dequeueAll: function (queueName) {
    var instance = this;
    var queue = this.queue;
    var allFns = (!queueName) ? this.queue : this.queue.filter(function (current) {
      return (current.name === queueName)
    });
    (function recursive(index) {
      var currentItem = allFns[index];
      if (!currentItem) return;
      currentItem.fn.call(instance, function () {
        queue.splice(queue.indexOf(currentItem), 1);
        recursive(index)
      })
    }(0));
    return this
  }
};

var myQueue = new Queue();
myQueue.enqueue(function (next) {
  console.log('D1: first test1');
  next()
}, 'first').enqueue(function (next) {
  setTimeout(function () {
    console.log('D1: first test2');
    next()
  }, 2000)
}, 'first').enqueue(function (next) {
  console.log('D1: second test1');
  next()
}, 'second').enqueue(function (next) {
  console.log('D1: second test2');
  next()
}, 'second').dequeueAll();
added 366 characters in body
Source Link
Joseph
Joseph
  • 25.2k
  • 2
  • 25
  • 37

You don't need 2 arrays to store the order of named queues. You can have one array, but each item is named. One advantage of this is that you only have one array to push to, and you keep the absolute order, regardless of name. And no more nested loops.

// Lets assume you have a queue like the one above, filtered optionally

(function recursive(index){
  // get the item at the index
  var currentItem = queue[index];

  // This is to check if there's no more functions in the queue
  if(!currentItem) return;

  //Otherwise, run providing the instance of your object, and the magic "next"
  currentItem.fn.call(instance,function(){
    //This function gets called when the current queued item is done executing
    
    // Splice off this item from the queue. We use splice since the
    // item we might be operating on is from a filtered set, not in the same
    // index as the original queue
    queue.splice(queue.indexOf(currentItem),1);

    // Run the next item
    recursive(++index);

  });

Here's how to use it:

 
// start with 0
}(0));
 

Here's how to use it:

// So you can have something synchronous
enqueue(function(next){
  //do something
  next();
});

// Or something asynchronous
enqueue(function(next){
  somethingAsync(function(){
    // done?
    next();
  });
});

#Fluent API

Consider fluent, aka "jQuery-like" APIs. It's pretty simple, just have the methods return the current instance, and that's it:

enqueue : function(){
  ...
  return this;
}

Then you can do something like jQuery and other libraries:

myQueue.enqueue(function(){...}).enqueue(function(){...}).enqueue(function(){...})

You don't need 2 arrays to store the order of named queues. You can have one array, but each item is named.

// Lets assume you have a queue like the one above, filtered optionally

(function recursive(index){
  // get the item at the index
  var currentItem = queue[index];

  // This is to check if there's no more functions in the queue
  if(!currentItem) return;

  //Otherwise, run providing the instance of your object, and the magic "next"
  currentItem.fn.call(instance,function(){
    //This function gets called when the current queued item is done executing
    
    // Splice off this item from the queue. We use splice since the
    // item we might be operating on is from a filtered set, not in the same
    // index as the original queue
    queue.splice(queue.indexOf(currentItem),1);

    // Run the next item
    recursive(++index);

  });

Here's how to use it:

// start with 0
}(0));
 
// So you can have something synchronous
enqueue(function(next){
  //do something
  next();
});

// Or something asynchronous
enqueue(function(next){
  somethingAsync(function(){
    next();
  });
});

You don't need 2 arrays to store the order of named queues. You can have one array, but each item is named. One advantage of this is that you only have one array to push to, and you keep the absolute order, regardless of name. And no more nested loops.

// Lets assume you have a queue like the one above, filtered optionally

(function recursive(index){
  // get the item at the index
  var currentItem = queue[index];

  // This is to check if there's no more functions in the queue
  if(!currentItem) return;

  //Otherwise, run providing the instance of your object, and the magic "next"
  currentItem.fn.call(instance,function(){
    //This function gets called when the current queued item is done executing
    
    // Splice off this item from the queue. We use splice since the
    // item we might be operating on is from a filtered set, not in the same
    // index as the original queue
    queue.splice(queue.indexOf(currentItem),1);

    // Run the next item
    recursive(++index);

  });
 
// start with 0
}(0));

Here's how to use it:

// So you can have something synchronous
enqueue(function(next){
  //do something
  next();
});

// Or something asynchronous
enqueue(function(next){
  somethingAsync(function(){
    // done?
    next();
  });
});

#Fluent API

Consider fluent, aka "jQuery-like" APIs. It's pretty simple, just have the methods return the current instance, and that's it:

enqueue : function(){
  ...
  return this;
}

Then you can do something like jQuery and other libraries:

myQueue.enqueue(function(){...}).enqueue(function(){...}).enqueue(function(){...})
Source Link
Joseph
Joseph
  • 25.2k
  • 2
  • 25
  • 37

#Naming

First off, I'd name my variables in the purpose they serve. Rather than ord, funcHash or even me.

#Queue adding and removing

You should remove the items that have executed from the queue. That's what a queue is all about, and that's what your code lacks at the moment.

Also, unshift and shift are more costly operations than 'pop' and 'push' because they need to move the array contents to new indices. You're correct in your implementation, that enqueue should use the unshift while execution uses pop.

#Collection of callbacks

You don't need 2 arrays to store the order of named queues. You can have one array, but each item is named.

function enqueue(fn,name){
  queue.push({
    name : name,
    fn : fn
  })
}

You can then use Array.prototype.filter to filter out the queue, if you want to only unload certain queued items with a certain name. Here's how you can do it:

function dequeue(name){
  var fns = queue.filter(function(current){
    return (current.name === name);
  });

  // pop-off and run
}

This code defaults to all items when name is not supplied

function dequeueAll(name){
  var fns = (!name) ? queue : queue.filter(function(current){
    return (current.name === name);
  });

  // pop-off and run all
}

Async control

Now, you don't have total control over the process that takes place in the queued function. The user could pop-in an async operation, and your library won't even know it, and will run wild. That's where your deferreds come in to play.

However, you don't really need deferreds. You can pass in a function to the queued function that calls the next function. This is similar to ExpressJS's middlewares, where there's a magical next function supplied to each middleware. Here's how it's implemented

// Lets assume you have a queue like the one above, filtered optionally

(function recursive(index){
  // get the item at the index
  var currentItem = queue[index];

  // This is to check if there's no more functions in the queue
  if(!currentItem) return;

  //Otherwise, run providing the instance of your object, and the magic "next"
  currentItem.fn.call(instance,function(){
    //This function gets called when the current queued item is done executing
    
    // Splice off this item from the queue. We use splice since the
    // item we might be operating on is from a filtered set, not in the same
    // index as the original queue
    queue.splice(queue.indexOf(currentItem),1);

    // Run the next item
    recursive(++index);

  });

Here's how to use it:

// start with 0
}(0));

// So you can have something synchronous
enqueue(function(next){
  //do something
  next();
});

// Or something asynchronous
enqueue(function(next){
  somethingAsync(function(){
    next();
  });
});

#Demonstration

Here's a small demo I have made here. It has some bugs in some corner cases and needs a few fixing. But given your code example, it works quite similarly. Not to mention, it is a bit shorter, and uses no libraries.