From scratch pub sub definition and implementation

Question

I am finished with my implementation of PUB/SUB or Observer pattern, however I just want to see any ways that I may be able to improve this pattern. I wanted to be able to use the typical PubSub pattern except add performance enhancements by using the new key word to instantiate each new instance of a PubSub object, as well as make sure that the initialize fn as well as all of the public methods are chain-able.

Here is my PubSub definition:

(function(global) {
'use strict';

var newPubSub;
global.PubSub = {
    core: function core() {},
    init: function(moduleNs) {
        if (!global[moduleNs]) global[moduleNs] = new PubSub.core();
        else throw new Error('A module has already been initialized under this namespace');
        newPubSub = global[moduleNs];
        return newPubSub;
    }
};

var instance = PubSub.core.prototype;

/**
 * Will check for the existance of prev array under this ns and if none exists it will create
 * a new prop with the ns and the function as the sole member of the array. If an array already
 * exists under this namespace then the function is simply pushed into that namespaces array of
 * methods that it will be triggered when that ns becomes published.
 **/
instance.sub = function(ns, fn) {
    if (!newPubSub[ns]) {
        newPubSub[ns] = [fn];
    }
    else if (Array.isArray(newPubSub[ns]) && newPubSub[ns].length > 0) {
        newPubSub[ns].forEach(function(savedFn) {
            if (fn === savedFn) throw new TypeError('This function already exists under this namespace, please provide a new method for this namespace or add a new namespace to hold this method/fn');
            else {
                newPubSub[ns].push(fn);
            }
        });
    }
    return this;
};

/**
 * Will remove all methods for a listener and the listener ns is not provided,
 * or will remove a single method from a listener if the ns provided has a matching method/fn
 **/
instance.unsub = function(ns, fn) {
    function revokeAllMethods(ns) {
            if (newPubSub[ns]) {
                delete newPubSub[ns];
            }
    }
    // loops through all the methods in a namespace provided and compares to see if the method provided exists, if it does it will be removed
    function revokeSpecificMethod(ns, fn) {
        newPubSub[ns].forEach(function(savedMethod, idx) {
            // Remove the method that matches the one provided to unsubscribe
            if (fn === savedMethod) newPubSub[ns].splice(idx, 1);
            // Otherwise an error will be thrown
            else throw new Error('No method match found under the namespace '+ns);
        });
    }
    if (typeof ns === 'string') {
        if (!fn) {
            revokeAllMethods(ns);
        }
        else if (typeof fn === 'function') {
            revokeSpecificMethod(ns, fn);
        }
    }
  return this;
};

/**
 * Will trigger an event with provided params on any methods under the ns triggered
 */
instance.pub = function(ns, params) {
    if (!newPubSub[ns]) throw new TypeError('No method found under the namespace "' + ns + '" inside of Object. It has either been unsubscribed or never existed.');

    newPubSub[ns].forEach(function(fn) {
        fn.apply(null, params);
    });
    return this;
};

})(window);

Implementation:

PubSub.init('test');

test.sub('alert', function(msg){
    alert(msg);
}).pub('alert',['bob']).sub('somethingelse', function(){
    alert('look ma no params');
}).pub('somethingelse').pub('alert',['bob']).unsub('alert').pub('alert',[12]);

Answer 1

if (!global[moduleNs]) global[moduleNs] = new PubSub.core();

I think this is a practice handed down from the OOP forefathers (Java, C++) where a singleton is created using a single instance of a class definition. In JS, you don't need to do this. Setting up the constructor, using new, putting methods etc is just unneeded code. You can simply use an object literal. Also, there's an alternate way to "use existing or create new global":

;(function(lib){
  // `lib` is an object which you can attach your API to.

  var events = {};

  lib.sub = function(){...};
  lib.unsub = function(){...};

  // This is the "use existing or create new"
}(this.PubSub = this.PubSub || {}));

---

Unless there is actually danger in proceeding, there is no need to throw errors all over the place. For instance, there is no harm in trying to subscribe a function that's already there, or unsubscribe a function that's not there. You can do a harmless log, but it's not something you should halt your app for.

---

Your sub can be better if it just received a function instead of a function or an array. You can always create a separate function for adding an array, which in turn, just calls sub for each item in the array. This avoid bloating your sub.

lib.sub = function(event, handler){
  if(typeof handler !== 'function') throw new Error('Handler not a function');
  // Use existing array or create an array for event name
  events[event] = events[event] || [];

  // We can use indexOf to check if the value is in the array.
  // If not, we push it there.
  if(!~events[event].indexOf(handler)) events[event].push(handler);
}

lib.subArray = function(event, handlers){
  handlers.forEach(function(handler){
    lib.sub(event, handler);
  });
}

---

unsub can be simplified too. It's just a matter of finding and removing the handler. You can either do loop and splice, or do a filter. Either way is costly, but for simplicity, I went for filter.

lib.unsub = function(event, handler){
  if(!events[event]) return;
  events[event] = events[event].filter(function(storedHandler){
    return storedHandler !== handler;
  });
}

lib.unsubArray = function(event, handlers){
  handlers.forEach(function(handler){
    lib.unsub(event, handler);
  });
}

lib.unsubAll = function(event){
  if(!events[event]) return;

  // A neat way to clear an array is to set its length to 0
  events[event].length = 0;
}

---

pub can be optimized. PubSub pattern is fire and forget. Fire some event, and the caller doesn't care who receives it. Thus, there is no need to throw an error if the event is in the registry or not.

Also, order of events should never be relied upon. Events can fire at any time, at any order by anyone. Knowing the order means you know who called, which means your code is coupled.

Taking advantage of this fact, we can use timers to fire our events instead of a loop. Handlers are third-party code, which are potentially long-running (like contain a 100k loop). The main advantage of doing so is to provide a gap between handler calls, giving other tasks like UI rendering, a chance to execute.

Regarding params, you can allow pub to accept normal args, not just one. You can just slice them after the event name onwards.

lib.pub = function(event){
  if(!events[event]) return;
  var params = Array.prototype.slice.call(arguments, 1);
  var handlerIndex = 0;
  var timer = setInterval(function(){

    // Using apply to send params. Events will receive them as args.
    // Also forcing context to null to avoid ambiguity of what `this` is
    events[event][handlerIndex].apply(null, params);

    // We clear the timer when done
    if(++handlerIndex === events[event].length) clearTimeout(timer);
  }, 0);
}

Answer 2

Error checking

Seems like you don't do all the error checking you'd need. For example, in this function

instance.sub = function(ns, fn) {
    if (!newPubSub[ns]) {
        newPubSub[ns] = [fn];
    }
    else if (Array.isArray(newPubSub[ns]) && newPubSub[ns].length > 0) {
        newPubSub[ns].forEach(function(savedFn) {
            if (fn === savedFn) throw new TypeError('This function already exists under this namespace, please provide a new method for this namespace or add a new namespace to hold this method/fn');
            else {
                newPubSub[ns].push(fn);
            }
        });
    }
    return this;
};

fn falls through the cracks if newPubSub[ns] is not an array but already exiss. Wouldn't you want to throw an error?

Similarly, you might also want to check that newPubSub[ns] has the expected form in unsub and that apply can be applied to each member of

Use closures to enforce encapsulation

All my concern about error checking relates to the fact that you are creating normal properties in these objects with your pub-sub methods, meaning that in the intervening time, a user can forget he was using a certain keyword with pub-sub and might use it for something else.

For example, here I corrupt your test variable by doing something entirely different with the alert key and forgetting I was using that for my pubsub functionality:

PubSub.init('test');

test.sub('alert', function(msg){
    alert(msg);
}).pub('alert',['bob'])

test['alert'] = "hi!"; // oops your 'alert' chain is gone

test.pub('alert', ['joe'])  //doesn't work

I think you'll want to restructure this to something like the following:

function pubsub() {
     var pub_arrays = {}
     return {
        sub: function(ns, fn){ 
                if(!pub_arrays[ns]) pub_arrays[ns] = [fn] 
                else p
                },
        pub:  .... and so on ....

    };
}

In this way all your methods will have access to the object that holds the publishing arrays (one array for each key word) but outside users won't have access to this so they won't be able to change it.

Variable scoping in JavaScript is not a strong suit of mine, but I think something like this, using closures for encapsulation, is where you want to be if you don't trust the user, or yourself, not to accidentally...or maliciously...mess with your pub/sub methods.

Additionally, it looks like the most recent version of JavaScript may provide Class methods to encapsulate data. Read more on that in these SO posts:

• https://stackoverflow.com/questions/29455692/how-does-closure-reify-the-data-encapsulation
• https://stackoverflow.com/questions/8729714/closures-vs-classes-for-encapsulation