Managing a queue in JavaScript via callbacks

Question

I am working on a page that uses JavaScript to manage a queue. My challenge is my code has nested callbacks. The nested callbacks are confusing me in regards to the scope of my queue. Currently, I have the following:

function MyApp() {}
module.exports = MyApp;

MyApp.myQueue = [];
MyApp.queueIsLocked = false;

MyApp.enqueue = function(item, onSuccess, onFailure) {
  if (!MyApp.queueIsLocked) {
    MyApp.queueIsLocked = true;        
    MyApp.myQueue.push(item);
    MyApp.queueIsLocked = false;

    item.send(   
      function() {
        console.log('item: ' + item.id);

        MyApp.queueIsLocked = true;                      
        MyApp.findItemById(item.id,
          function(index) {
            if (index !== -1) {
              MyApp.myQueue.splice(index, 1);
              MyApp.queueIsLocked = false;

              if (onSuccess) {
                onSuccess(item.id);
              }
            }
          }
        );
      },
      function() {
        alert('Unable to send item to the server.');
        if (onFailure) {
          onFailure();
        }
      }
    );
  }
};

MyApp.findItemById = function(id, onComplete) {
  var index = -1;
  if (MyApp.queueIsLocked) {
    setTimeout(function() {
      // Attempt to find the index again.
    }, 100);
  } else {
    MyApp.queueIsLocked = true;
    for (var i=0; i<MyApp.myQueue.length; i++) {
      if (MyApp.myQueue[i].id === id) {
        index = i;
        break;
      }
    }
  }

  if (onComplete) {
    onComplete(index);
  }
};

The send function behaves differently based on the details of item. Sometimes the item will be sent to one server. Sometimes, it will be sent to multiple servers. Either way, I do not know when the item will be done being "sent". For that reason, I'm using a callback to manage the queue. When the item is done being "sent", I want to remove it from the queue. I need to use either a timeout or interval to check to see if the queue is locked or not. If its not locked, I want to remove the item from the queue. This check is adding another level of nesting that is confusing me.

My challenge is, I do not believe that the scope of index is working like I expected. I feel like I'm getting a race condition. I'm basing this on the fact that I've written the following Jasmine test:

describe('Queue', function() {
  describe('Approach 1', function() {
    it('should do something', function() {
      MyApp.enqueue({id:'QRA', text:'Test A'});
    });
  });

  describe('Approach 2', function() {
    it('should successfully queue and dequeue items', function() {
      MyApp.enqueue({id:'WX1', text:'Test 1'});
      MyApp.enqueue({id:'QV2', text:'Test 2'});
      MyApp.enqueue({id:'ZE3', text:'Test 3'});
    });
  });
});

When I execute the test, I see the following in the console window:

item: QRA index: 1
item: WX1 index: 2
item: QV2 index: 3
item: ZE3 index: 4

Its like the items aren't getting dequeued like I would expect. Am I way off base in my approach of managing a queue? What am I doing wrong?

Thank you for any assistance.

Why would you ever need to check queueislocked? I don't see anywhere in your code where it is necessary. — slebetman
– slebetman, Commented Aug 19, 2015 at 13:53

GreenGiant · Accepted Answer · 2015-08-19 14:58:01Z

Here are some questions you need to think through and answer for yourself about your intent and design:

It sounds like the queue represents items you are trying to send to the server. You are adding items to the queue that need to be sent, and removing them from the queue after they have been successfully sent.
Do you want your code to send multiple items simultaneously, in parallel? For example, item A is added to the queue, then sent. Before the asynchronous send for A finishes, item B is added to the list. Should the code try to send item B before the send of item A finishes? Based on your code, it sounds like yes.

It seems that you don't really want/need a queue, per se, so much as you want a list to track which items are in the process of being sent. "Queue" implies that objects are being processed in some kind of FIFO order.

If you just want to track items based on id, then you can use an object instead. For example:

MyApp.items = {};
MyApp.addItem = function(item){
  MyApp.items[item.id] = item;
  item.send(
    function(){ // success
      MyApp.removeItem(item.id)
    }
  );
}
MyApp.removeItem = function(id){
  delete MyApp.items[id];
  onSuccess(id);
}

Also, I don't think you need a lock on the queue. Javascript is single-threaded, so you'll never have a case where two parts of your code are trying to operate on the queue at the same time. When an ajax call finishes asynchronously, your callback code won't actually be executed until any other code currently executing finishes.

Wasmoo · Accepted Answer · 2015-08-19 20:43:44Z

The big flaw I'm seeing is that you call MyApp.queueIsLocked = true immediately before MyApp.findItemById. Because it's locked, the function sets up a timeout (that does nothing), and proceeds to call onComplete(-1). -1 is then explicitly ignored by onComplete, failing to dequeue, and locking your queue.

You probably meant to retry the find, like this:

setTimeout(function() {
  // Attempt to find the index again.
  MyApp.findItemById(id, onComplete);
}, 100);

I'm not sure, but I think Jasmine requires explicit instruction to get Timeout functions to fire, using jasmine.clock().tick

That said, I suggest removing all of the references to queueIsLocked, including the above timeout code. Also, if item.id is always a unique string, you can use an object instead of an array to store your values.

Here is a suggested iteration, staying as true to the original API as possible:

function MyApp() {}
module.exports = MyApp;

MyApp.myQueue = {};

//Sends the item, calling onSuccess or onFailure when finished
//  item will appear in MyApp.myQueue while waiting for a response from send
MyApp.enqueue = function(item, onSuccess, onFailure) {
  MyApp.myQueue[item.id] = item;

  item.send(function() {
    console.log('item: ' + item.id);
    delete MyApp.myQueue[item.id];
    if (onSuccess) {
      onSuccess(item.id);
    }
  }, function() {
    alert('Unable to send item to the server.');
    if (onFailure) {
      onFailure();
    }
  });
};

//Returns the Item in the queue, or undefined if not found
MyApp.findItemById = function(id, onComplete) {
  if (onComplete) {
    onComplete(id);
  }
  return MyApp.myQueue[id];
};

Alex Nikulin · Accepted Answer · 2015-08-27 07:41:03Z

Try to use ECMA 6 Promise or any promise from js framework.Promiseis more suitable for this task. see more at https://developer.mozilla.org/

function MyApp() {}
module.exports = MyApp;

MyApp.myQueue = [];
MyApp.queueIsLocked = false;

MyApp.enqueue = function(item) {
 return new Promise(function(resolve, reject) {
      if (!MyApp.queueIsLocked) {
        MyApp.queueIsLocked = true;        
        MyApp.myQueue.push(item);
        MyApp.queueIsLocked = false;

        var onResolve = function() {
            console.log('item: ' + item.id);
            MyApp.queueIsLocked = true;   
            MyApp.findItemById(item.id).then(function(index){
                 if (index !== -1) {
                      MyApp.myQueue.splice(index, 1);
                      MyApp.queueIsLocked = false;
                      resolve(item.id);
                 }
            });     

        };

        item.send(onResolve,reject);
      }
  });
};

MyApp.findItemById = function(id) {
     return new Promise(function(resolve, reject) {
              var index = -1;
              if (MyApp.queueIsLocked) {
                setTimeout(function() {
                  // Attempt to find the index again.
                }, 100);
              } else {
                MyApp.queueIsLocked = true;
                for (var i=0; i<MyApp.myQueue.length; i++) {
                  if (MyApp.myQueue[i].id === id) {
                    index = i;
                    break;
                  }
                }
                resolve(index);
              }
        });
};

Jacek Pietal · Accepted Answer · 2015-08-26 13:43:28Z

0

move MyApp.queueIsLocked = false; to the callback of server send

answered Aug 26, 2015 at 13:43

Jacek Pietal

2,0191 gold badge20 silver badges28 bronze badges

Collectives™ on Stack Overflow

Managing a queue in JavaScript via callbacks

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related