Function queue for synchronous execution

Question

The purpose of this code is to allow to add functions to a queue, that will execute them sequentially.

With this in mind, it also allows to execute them with a defined delay, to prevent them from blocking the browser.

(function(window, undefined){
    'use strict';
    var queue = [];
    var running = false;
    var stop = false;
    var log = function(data){
        //some browsers may not have the .error method
        (window.console.error || window.console.log).call(window.console, data);
    };
    var runNext = function(){
        //I have to do like this, so I don't insert it back
        var fn = queue[0];
        setTimeout(function(){
            try
            {
                //sometimes, when it is to stop, it's already here
                if(!stop)
                {
                    queue.shift();//remove it from the queue
                    fn.fn();
                }
            }
            catch(e)
            {
                //simply log it, to give a chance to other functions
                log(e);
            }

            if(queue.length && !stop)
            {
                setTimeout(runNext, 1);
            }
            else
            {
                running = false;
            }
        }, fn.delay);
    };

    window.FnQueue = {
        add: function(fn, delay){
            if(!(fn instanceof Function))
            {
                log('FnQueue.add - fn must be a function');
                return window.FnQueue;
            }
            if(isNaN(delay) || delay < 1)
            {
                log('FnQueue.add - delay must be a number equal or higher than 1');
                return window.FnQueue;
            }
            queue.push({
                fn:fn,
                delay:delay
            });
            //starts the execution only if it wasn't running and stopped
            if(!running && !stop) 
            {
                running = true;
                setTimeout(runNext, 1);
            }
            return window.FnQueue;
        },
        clear: function(){
            queue = [];
            return window.FnQueue;
        },
        stop: function(){
            stop = true;
            return window.FnQueue;
        },
        resume: function(){
            stop = false;
            if(!running && queue.length)
            {
                setTimeout(runNext, 1);
            }
            return window.FnQueue;
        },
        getQueue: function(){
            return queue;
        },
        isRunning: function(){
            return running && !stop;
        }
    };
})(Function('return this')());

What can I improve on my code to make it less spaghetti-ish and more distributed? What inefficient/unnecessary code can I improve?

Answer 1

While loging to the console is a good idea, this doesn't give other code using this library a lot of control over errors; how is other code supposed to know when something went wrong?

I recommend throwing (custom?) errors along with logging a message to the console so, along with the person using the library being able to see and debug the error, their code will also be able to see and interact with the error.

In case you aren't familiar with Java, you can create a custom error just like you can create any other class.

Here is an example using your code:

function BadDelayValueException(delay) {
    this.message = "Delay must be a valid number greater than or equal to 1.";
    this.delay = delay;
    this.toString = function() {
        return this.message + " Delay: " + this.delay;
    }
}

Then, in your code, you can write this:

if(isNaN(delay) || delay < 1)
{
    log('FnQueue.add - delay must be a number equal or higher than 1');
    throw new BadDelayValueException(delay);
}

Now, any code that is using this library that you created can use a simple try/catch exception handler to watch out for this error that you are throwing.

To repeat, by adding this error throwing, code that is using this library can now interact (and perhaps automate usage of in some sort of way) with the errors thrown from this library.

For more information about custom errors and error throwing and catching, read this MDN article.

Answer 2

With this in mind, it also allows to execute them with a defined delay, to prevent them from blocking the browser.

This is a common misconception of how the timers work. Currently executing code doesn't stop when a timer expires. That callback waits until the currently running code finishes.

You can think of JS as having 3 internal data structures:

1. The execution stack, where the currently running code is.
2. The callback queue, where all callbacks for async operations are stored
3. The callback ready queue, where all callbacks for competed async operations are stored.

When you call a timer function, that callback is stored in the callback queue until the timer expires. When it does, it moves to the ready queue, but it doesn't execute just yet. The current code in the stack will continue until it exhausts the stack. Only then will the engine pick from the ready queue and execute it. This is what they call the "event loop" which is basically code taking turns, one after the other.

So adding a delay isn't really needed. You can even put a delay of 0 and it won't block the UI.

What does block the UI is when operations in the stack take too long - regardless where they came from. If you happen to call a function that iterates through 10 billion items, that's the problem. Your stack won't exhaust fast enough to let other code run.

---

(window.console.error || window.console.log).call(window.console, data);

This is a problem, because some browsers don't even have a console object (I'm looking at you IE8). You'll probably get a "Accessing property error of undefined) when you run this piece of code. Check for console first to be safe.

---

try {
  //sometimes, when it is to stop, it's already here
  if(!stop){
    queue.shift();//remove it from the queue
    fn.fn();
  }
}

There are 2 problems in this piece of code. First is why are you even trying to use try-catch. Whenever I see a try-catch, it's an indicator that the developer is unsure of what will happen with the code. Solve the root problem of why the code can be unpredictable and make it predictable, before resorting to a try-catch.

The next is optimization. Engines like V8 optimize code heavily (and which is why Chrome is fast). However, code inside a try-catch doesn't get optimized because it is deemed unpredictable.

So what you can do is place all of your code in the try-catch inside a function and call that function in the try-catch instead. That way, the engine can optimize the function, while what doesn't get optimized is the call.

---

setTimeout(runNext, 1);

I assume your function queue isn't sensitive to when it gets called due to this. Anyways, the problem with this code is that it's calling setTimeout all the time. The problem here is that, as explained in the first section of the answer, you are scheduling when you call a timer.

A better solution for this is to schedule once, using setInterval. Each iteration will call the function next in line. Once the queue is done, you can clear the interval using clearInterval.

---

Function('return this')()

While this relatively looks safe (because the string isn't arbitrary), you will run into issues when using this piece of code in environments with very strict CSP rules. This includes mobile devices, browser extensions and some websites who lock down these things for security.

---

getQueue: function(){
  return queue;
},

I wouldn't want one to manipulate the queue outside if I were you. It could mess up your code. I suggest you return a copy of the queue, and not the queue array itself. Use slice to return a copy of the queue.

---

clear: function(){
  queue = [];
  return window.FnQueue;
},

A better way to clear an array without creating another array is to just set the length to zero.

---

If I understood the code correctly and following my suggestions, the following should be a simple replication:

;(function(ns){

  var queue = [];
  var isRunning = false;
  var runner = null;

  ns.add = function(fn){
    queue.push(fn);
    if(!isRunning) ns.resume();
    return ns;
  };

  ns.clear = function(){
    queue.length = 0;
    return ns;
  };

  ns.stop = function(){
    clearInterval(runner);
    runner = null;
    isRunning = false;
    return ns;
  };

  ns.resume = function(){
    if(!isRunning && !runner){
      isRunning = true;
      runner = setInterval(function(){
        // Here, I intentionally set the context to null. This makes the
        // callbacks have a `this` value of null, instead of `window`.
        queue.shift().call(null);
        if(!queue.length) ns.stop();
      }, 0);
    }
    return ns;
  };

  ns.getQueue = function(){
    return queue.slice(0);
  };

  ns.isRunning = function(){
    return isRunning;
  };

}(this.FnQueue = this.FnQueue || {}));

// Sample usage

alert('Let us start. This is your first alert in the stack');

FnQueue.add(function(){
  alert('First scheduled item');  
});

alert('This should fire second, we are still in the stack');


FnQueue.add(function(){
  alert('Second scheduled item');  
});


FnQueue.add(function(){
  alert('Third scheduled item');  
});


FnQueue.add(function(){
  alert('Fourth scheduled item');  
});


alert('After this alert, the stack is empty. All callbacks should fire in sequence');