Change channel priority sort

This fixes issue #15 where a subscriber with an initial priority of 9
winds up being called after a subscriber with priority 1 which was later
added.

Instead of manually splicing the subscriber into the array of
subscribers the `addSubscriber` and `setPriority` and changing the value
of the priority property, subscribers are now sorted using the `sort`
function of the array with a custom compare function.

This compare function will sort the subscribers based on their `priority`
property on the `options` object in descending order. The custom sort
method doesn't not manipulate the value of the priorityproperty giving a
much more predictable result.

The unit tests for the priorities have been update to better test if the
sort works as expected. An additional unit test specifically for
issue #15 has been added.

Author: Thijs Busser

lib/mediator.js

@@ -98,24 +98,39 @@
   // through the Mediator instance.
 
   Channel.prototype = {
-    addSubscriber: function(fn, options, context){
-      var subscriber = new Subscriber(fn, options, context);
+    // Sort function for sorting the channels based on their priority. The sort will
+    // place the channel with the highest priority in the first place of the array,
+    // all other channels will be added in descending order. i.g. a channel with a
+    // priority of 9 will be placed before a channel with a priority of 3
+    channelPrioritySort: function(a, b) {
+      if (a.options.priority > b.options.priority) {
+        return -1;
+      } else if (a.options.priority < b.options.priority) {
+        return 1;
+      }
+      return 0;
+    },
 
-      if(options && options.priority !== undefined){
+    addSubscriber: function(fn, options, context){
+      // Make sure there is always an options object
+      if (options == null) {
+        options = {};
+      }
+      if (options.priority == null) {
+        // If priority is undefined we will add a default priority of 0
+        options.priority = 0;
+      } else {
         // Cheap hack to either parse as an int or turn it into 0. Runs faster
         // in many browsers than parseInt with the benefit that it won't
         // return a NaN.
-        options.priority = options.priority >> 0;
-
-        if(options.priority < 0){ options.priority = 0; }
-        if(options.priority >= this._subscribers.length){ options.priority = this._subscribers.length-1; }
-
-        this._subscribers.splice(options.priority, 0, subscriber);
-      }else{
-        this._subscribers.push(subscriber);
+        options.priority = options.priority | 0;
       }
 
+      var subscriber = new Subscriber(fn, options, context);
       subscriber.channel = this;
+      this._subscribers.push(subscriber);
+
+      this._subscribers.sort(this.channelPrioritySort);
 
       return subscriber;
     },
@@ -143,23 +158,17 @@
     // an array index. It will not search recursively through subchannels.
 
     setPriority: function(identifier, priority){
-      var oldIndex = 0,
-          x = 0,
-          sub, firstHalf, lastHalf, y;
+      var x = 0,
+          y = this._subscribers.length;
 
-      for(x = 0, y = this._subscribers.length; x < y; x++){
-        if(this._subscribers[x].id === identifier || this._subscribers[x].fn === identifier){
+      for(; x<y; x++) {
+        if (this._subscribers[x].id === identifier || this._subscribers[x].fn === identifier){
+          this._subscribers[x].priority = priority;
           break;
         }
-        oldIndex ++;
       }
 
-      sub = this._subscribers[oldIndex];
-      firstHalf = this._subscribers.slice(0, oldIndex);
-      lastHalf = this._subscribers.slice(oldIndex+1);
-
-      this._subscribers = firstHalf.concat(lastHalf);
-      this._subscribers.splice(priority, 0, sub);
+      this._subscribers.sort(this.channelPrioritySort);
     },
 
     addChannel: function(channel){

test/ChannelSpec.js

@@ -67,15 +67,14 @@ describe("Channel", function() {
     it("should be able to set top priority", function(){
       var spy = sinon.spy(),
           spy2 = sinon.spy(),
-          spy3 = sinon.spy();
-
-      channel.addSubscriber(spy);
-      channel.addSubscriber(spy2);
-      channel.addSubscriber(spy3, { priority: 1 });
-
-      expect(channel._subscribers[0].fn).to.equal(spy);
-      expect(channel._subscribers[1].fn).to.equal(spy3);
-      expect(channel._subscribers[2].fn).to.equal(spy2);
+          spy3 = sinon.spy(),
+          idA = channel.addSubscriber(spy, { priority: 1}).id,
+          idB = channel.addSubscriber(spy2).id,
+          idC = channel.addSubscriber(spy3, { priority: 3 }).id;
+
+      expect(channel._subscribers[0].id).to.equal(idC);
+      expect(channel._subscribers[1].id).to.equal(idA);
+      expect(channel._subscribers[2].id).to.equal(idB);
     });
 
     it("should be able to set arbitrary priority", function(){
@@ -87,26 +86,34 @@ describe("Channel", function() {
       channel.addSubscriber(spy2);
       channel.addSubscriber(spy3, { priority: 1 });
 
-      expect(channel._subscribers[0].fn).to.equal(spy);
-      expect(channel._subscribers[1].fn).to.equal(spy3);
+      expect(channel._subscribers[0].fn).to.equal(spy3);
+      expect(channel._subscribers[1].fn).to.equal(spy);
       expect(channel._subscribers[2].fn).to.equal(spy2);
     });
 
     it("should be able to change priority after adding it", function(){
       var spy = sinon.spy(),
           spy2 = sinon.spy(),
-          spy3 = sinon.spy();
+          spy3 = sinon.spy(),
+          idA = channel.addSubscriber(spy, { num: 1 }).id,
+          idB = channel.addSubscriber(spy2, { num: 2 }).id,
+          idC = channel.addSubscriber(spy3, { num: 3 }).id;
 
-      var sub = channel.addSubscriber(spy, { num: 1 });
-      channel.addSubscriber(spy2, { num: 2 });
-      channel.addSubscriber(spy3, { num: 3 });
+      channel.setPriority(idA, 2);
 
-      channel.setPriority(sub.id, 2);
+      expect(channel._subscribers[0].id).to.equal(idA);
+      expect(channel._subscribers[1].id).to.equal(idB);
+      expect(channel._subscribers[2].id).to.equal(idC);
 
-      expect(channel._subscribers[0].fn).to.equal(spy2);
-      expect(channel._subscribers[1].fn).to.equal(spy3);
-      expect(channel._subscribers[2].fn).to.equal(spy);
+    });
+    it("should keep the priorities as initially set (issue #15)", function() {
+      var spy = sinon.spy(),
+          spy2 = sinon.spy(),
+          idA = channel.addSubscriber(spy, {priority: 9}).id,
+          idB = channel.addSubscriber(spy, {priority: 1}).id;
 
+      expect(channel._subscribers[0].id).to.equal(idA);
+      expect(channel._subscribers[1].id).to.equal(idB);
     });
   });
 
@@ -184,13 +191,13 @@ describe("Channel", function() {
       var spy = sinon.spy(),
           spy2 = sinon.spy(),
           spy3 = sinon.spy();
-      
+
       channel.addSubscriber(spy);
       var sub2 = channel.addSubscriber(spy2);
       expect(channel._subscribers.length).to.equal(2);
       channel.addSubscriber(spy3);
       expect(channel._subscribers.length).to.equal(3);
-      
+
       channel.removeSubscriber(sub2.id);
       expect(channel._subscribers.length).to.equal(2);
       expect(channel._subscribers[0].fn).to.equal(spy);
@@ -268,7 +275,7 @@ describe("Channel", function() {
       expect(spy).calledWith(data[0]);
       expect(spy2).calledWith(data[0]);
     });
-    
+
     it("should call all matching subscribers with context", function(){
       var spy = sinon.spy(),
           data = ["data"];
@@ -278,7 +285,7 @@ describe("Channel", function() {
 
       expect(spy).called;
     });
-    
+
     it("should call subscribers in predefined priority", function(){
       var sub1 = function(){
         this.a += "1";

test/MediatorSpec.js

@@ -268,7 +268,7 @@ describe("Mediator", function() {
           sub = mediator.subscribe("test", spy),
           sub2 = mediator.subscribe("test", spy2);
 
-      sub2.update({ options: { priority: 0 } });
+      sub2.update({ options: { priority: 1 } });
 
       expect(mediator.getChannel("test")._subscribers[0].id).to.equal(sub2.id);
       expect(mediator.getChannel("test")._subscribers[1].id).to.equal(sub.id);