Revealing module implicity calling an internal function - is this a “smell” I'm trying to improve my javascript and have been using the revealing module pattern to good effect but I noticed that code in the module not in a function or the return is executed on creation have utilised this, such as:

var MyModule = function ($) {

    setUp();

    function setUp() {
        // execute code to be run when module created
    }

    function doStuff() {
    }

    return {
        doStuff: doStuff
    }
}


I can see this is creating a form of constructor for the function which relies on it being called by var myModule = new MyModule(jQuery).

Any feedback on the rights or wrongs of this appreciated.
In this case, you don't actually use the new keyword. It's valid to return something from a constructor, since constructors are functions as well. But when you use new to construct objects, it should return an instance of that constructor. In your case, you are returning an object, one that is not tied in any way to the constructor.

Here's the revealing module pattern and how it's used:

function createModule(name){

  //your private members, enclosed in createModule
  var privateVar = name;

  function privateFn(){
    console.log(privateVar);
  }

  //return a newly created object as the module
  return {
    //here, we have a function
    //since we live in the same scope as the privates, we can
    //also emulate getters and setters, classical OOP style
    getName : function(){
      privateFn();
    },
    setName : function(name){
      privateVar = name;
    }
  }
}

var newModule = createModule('mymodule');
newModule.getName(); //mymodule
newModule.setName('newname');
newModule.getName(); //newname


Now there are issues with this pattern, especially in usage:

Pros


It emulates private members. This would be an easy start for developers coming from Classical OOP.
Since the members are directly attached in the module, or at least only one scope above in the closure, they are fast.
No prototype chains to worry about.


Cons


Creating multiple modules mean creating multiple objects, each with it's own scope and functions. One module's getName function is not the same as another module's getName(). Although perfectly fine (And that's how classical OOP works, as far as I know), it's bad for the browser since it uses too much memory.
You can't use instanceof to determine the modules since they all are instances of the Object object, and they are all object literals ({}) to begin with.


Best use

Single global modules, like libraries.



For spawning many modules, I suggest the native way of doing it, and would be creating instances using constructors and prototypes.

function Module(name){
  this.name = name;
}

createModule.prototype.getName = function(){
  console.log(this.name);
}

var newModule = new Module('mymodule');
newModule.getName(); //mymodule
newModule.name = 'newname';
newModule.getName(); //newname


It has its own issues as well:

Pros


Smaller memory footprint. Anything attached to the constructor's prototype is shared, not duplicated, across instances. This means one instance's getName is the same function as another instance's getName. This is because of the prototype chain (think of it as inheritance chain), where instances "inherit" the same object.


Cons


No private scope
A long prototype chain is a performance penalty, especially if the method lives far up the chain.
Overkill for a single module. It was meant to spawn multiple instances.


Best Use

Spawning multiple instances of the same kind, like elements in a game (units, enemies, bullets).



What should you use:

Depends. Libraries usually use a mix as well. 

Take for instance, jQuery. The library itself is a module. They are revealed into the global scope using a modified revealing module pattern called "exports/expose pattern". Creation of instances of the jQuery object uses new, hidden behind the jQuery function.

(function(window,undefined){

  jQuery = function( selector, context ) {
    //creating a new instance here
    return new jQuery.fn.init( selector, context, rootjQuery );
  }

  //...tons of library code here...

  //Expose, assign to window, basically creating the globals
  window.jQuery = window.$ = jQuery;

}(window));


When using jQuery as an object, it hosts exposed functions. This is when the module pattern is good at - singe direct fast global access.

$.ajax
$.noConflict
$.each


But when you use jQuery to wrap elements, wrap objects etc., it creates an instance of the constructor. This is where the constructor pattern is good at - creating multiple objects of the same kind.

//methods on these selectors are basically the same function, shared
// where in this case, each is shared
$('html').each(...
$('body').each(...
$('head').each(...

$('head').each === $('body').each //true