1
\$\begingroup\$

I recently had an interesting requirement in a test environment where I needed to simulate actions at varied (random) intervals in a node back-end. They needed to be sequentially executed, but queued so I could fire and move on. Using a typical timeout/interval here wouldn't work, since if I set a timeout for fn1 to execute after 500ms and then set a timeout for fn2 to execute after 250ms, fn2 would execute before fn1, which was not desired.

To solve this, I created a queue object that executes queued commands sequentially. It works perfectly for my scenario, but I was wondering if there were any opportunities for improvement. I realize I should probably be checking input parameters to make sure the passed in function is indeed a function, as well as the delay being a number. Maybe there are other opportunities for improvement. Let me know!

var TimerQueue = (function () {
    var timers = [];
    var running = false;
    var currentInterval;
    var currentTimer;

    this.addTimer = function (fn, delay) {
        timers.push({fn: fn, delay: delay});

        function exec() {
            currentTimer.fn();
            clearInterval(currentInterval);
            if (timers.length > 0) {
                currentTimer = timers.shift();
                currentInterval = setInterval(exec, currentTimer.delay);
            } else {
                running = false;
            }
        }

        if (!running) {
            running = true;
            currentTimer = timers.shift();
            currentInterval = setInterval(exec, currentTimer.delay);
        }
    };

    this.clear = function () {
        if (currentInterval) {
            clearInterval(currentInterval);
        }
        timers = [];
        running = false;
    };

    return this;
});

Usage:

var queue = new TimerQueue();
queue.addTimer(function () { console.log('1st') }, 500);
queue.addTimer(function () { console.log('2nd') }, 250);
\$\endgroup\$
  • \$\begingroup\$ Did you want the queues to resolve asynchronously? I.e. if you enqueue a function with a 500 timer and a 250 timer, should the second function wait 750 ms or 500? \$\endgroup\$ – megawac Nov 28 '16 at 18:06
  • \$\begingroup\$ The second timer's delay should start when the first one's ends. So in that case the second should wait a total of 750ms. \$\endgroup\$ – pulse0ne Nov 28 '16 at 18:16
1
\$\begingroup\$

You can still use setTimeout. You just need to wrap it in a function and recursively call that function from within the timer callback while tasks still exist in the queue.

Since what you need to do is schedule a task to run once after a delay, setTimeout is a better choice. Remember that setInterval runs the callback multiple times. If you don't clear it at the right time, you might face a callback running more than once.

The presence of the timer ID is an indicator that a task is scheduled to run. No need for a running flag. Additionally, clear timer functions don't null references, you need to do it manually.

A quick way to clear an array without assigning it another array is to set its length to 0.

Prefer just using a function declaration when creating a constructor. It's simpler. Also, there's little benefit using "private" variables. It only forces you to define methods on the instance instead of the prototype which makes it not ideal. Methods should be on the prototype.

Here's my take on it. Untested, but should work the same in theory:

function TimerQeueue(){
  this.currentTimer = null;
  this.tasks = [];
}

TimerQueue.prototype.addTask = function(callback, delay){
  this.tasks.push({ callback: callback, delay: delay });

  // If there's a scheduled task, bail out.
  if(this.currentTimer) return;

  // Otherwise, start kicking tires
  this.launchNextTask();
};

TimerQueue.prototype.launchNextTask = function(){

  // If there's a scheduled task, bail out.
  if(this.currentTimer) return;

  var self = this;
  var nextTask = this.tasks.shift();

  // There's no more tasks, clean up.
  if(!nextTask) return this.clear();

  // Otherwise, schedule the next task.
  this.currentTimer = setTimeout(function(){
    nextTask.callback.call();

    self.currentTimer = null;

    // Call this function again to set up the next task.
    self.launchNextTask();
  }, nextTask.delay);
};

TimerQueue.prototype.clear = function(){
  if (this.currentTimer) clearTimeout(this.currentTimer);

  // Timer clears only destroy the timer. It doesn't null references.
  this.currentTimer = null;

  // Fast way to clear the task queue
  this.tasks.length = 0;
};
\$\endgroup\$
  • 1
    \$\begingroup\$ I really like this approach. I did have to change this.currentTimer = null to self.currentTimer = null in the setTimeout() call, but other than that, it works great. \$\endgroup\$ – pulse0ne Nov 28 '16 at 22:10
  • \$\begingroup\$ I have no idea why I used setInterval over setTimeout in my original code. And thanks for the advice on setting the array length to 0 to clear it! \$\endgroup\$ – pulse0ne Nov 28 '16 at 22:18
4
\$\begingroup\$

I would do it as a singleton, so that the core functionality can be protected and private. All that is exposed is the Interface, .add(),start(),clear(),and the semaphore .done

Using prototype is only an advantage if the object that it belongs to is instanced many times. If you only create the object a few times, or once then adding to the prototype is actually more work for the javascript engine.

I also added a start method so that the queue could be created and started when needed. Once started you can add to the queue as long as done === false

// singleton. Assuming you only need one copy of timed queue
const timedQueue = (function(){
    var API;                // internal referance to interface
    const queue = [];       // array to hold functions
    var task = null;        // the next task to run
    var tHandle;            // To stop pending timeout 
    function next(){  // runs current scheduled task and  creates timeout to schedule next
        if(task !== null){          // is task scheduled??
            task.func();            // run it
            task = null;            // clear task
        }
        if(queue.length > 0){       // are there any remain tasks??
            task = queue.shift();   // yes set as next task
            tHandle = setTimeout(next,task.time) // schedual when
        }else{
            API.done = true;
        }
    }
    return API = {
        add : function(func,time){
            queue.push({func : func, time: time});
        },
        start : function(){
            if(queue.length > 0 && API.done){
                API.done = false;   // set state flag
                tHandle = setTimeout(next,0);
            }
        },
        clear : function(){
            task = null;            // remove pending task
            queue.length = 0;       // empty queue
            clearTimeout(tHandle);  // clear timeout
            API.done = true;        // set state flag
        },
        done : true,
    }
})();


// usage.
function test1(){
    console.log("Test1 run");
}
function test2(){
    console.log("Test2 run");
}

timedQueue.add(test1,1000)
timedQueue.add(test2,1000)
timedQueue.add(test1,3000)
timedQueue.add(test2,100)
timedQueue.start();
\$\endgroup\$
  • \$\begingroup\$ This is an elegant solution as well. This is more in line with my original thoughts, but definitely cleaner. I like the 'next' function and its name. I did need multiple separate queues, but your snippet is modified easily enough to allow for that. Thank you for the submission! Upvoted \$\endgroup\$ – pulse0ne Nov 29 '16 at 17:04
  • \$\begingroup\$ @pulseOne to convert to multi instanced just remove the immediate invocation const timedQueue = (function(){ ... code })() becomes const timedQueue = function(){ ... code } and you can then create them as needed with var myQueue = timedQueue(); \$\endgroup\$ – Blindman67 Nov 29 '16 at 17:45
0
\$\begingroup\$

Typescript Implementation

interface TimedJob{
    callback: Function;
    time: number;
}
class TimedQueue{
    private queue: Array<TimedJob> = [];
    private tHandle: any = null;
    private done: boolean = true;
    constructor() {
    }

    addTask(job:TimedJob):number {
        if (!this.done)
            throw new Error('Job is under process');
        return this.queue.push(job);
    }

    removeTask(job:TimedJob) {
        if (!this.done)
            throw new Error('Job is under process');
        let index = this.queue.indexOf(job);
        if (index == -1)
            throw new Error('Job not found');
        this.queue.splice(index, 1);
    }

    start() {
        if (!this.done)
            throw new Error('Job already started');
        if (this.queue.length == 0)
            throw new Error('Job Queue is empty');
        this.done = false;
        this.tHandle=setTimeout(()=>{ this.next() }, 0);
    }

    private next() {
        if (this.queue.length == 0)
        {
            this.done = true;
            return;
        }
        let job: TimedJob = this.queue.shift();
        job.callback.call(null);
        this.tHandle=setTimeout(()=>{ this.next() }, job.time);
    }

    stop() {
        if (!this.done)
            throw new Error('Queue is not started');
        clearTimeout(this.tHandle);
    }

    reset() {
        clearTimeout(this.tHandle);
        this.tHandle = null;
        this.done = true;
        if(this.queue.length>0)
            this.queue.splice(0, this.queue.length);
    }
}

Run the code

let job: TimedQueue = new TimedQueue();
job.addTask({
    callback: () => { console.log('1 second'); },
    time:1000
});
job.addTask({
    callback: () => { console.log('2 second'); },
    time:2000
});
job.addTask({
    callback: () => { console.log('3 second'); },
    time:3000
});

job.start();
\$\endgroup\$
  • 1
    \$\begingroup\$ It would be useful if you describe what you have improved. \$\endgroup\$ – Billal Begueradj Nov 27 '17 at 11:03
  • \$\begingroup\$ Nothing new compared to last post. It is just typescript implementation with reset functionality and it is multiple instance \$\endgroup\$ – Deepak Kumar T P Nov 27 '17 at 11:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.