Is this a keyword in a module template?

Question

I just started working in a new company and noticed something completely wrong for me in many of my JSs. I am a little hesitant to pick it up without confirming that it is wrong, since I am quite young, I am not a JS expert, and this is only my second day and I do not want to look stupid.

So, usually I expect the module template to look something like this:

MODULENAME = MODULENAME || {}; MODULENAME.SUBMODULENAME = (function() { var bla = {}; bla.somefunction = function() { //do stuff }; //add more stuff to bla return bla; }()); 

What do they have in all of their code:

 MODULENAME = MODULENAME || {}; MODULENAME.SUBMODULENAME = (function() { var that = this; that.somefunction = function() { //do stuff }; //add more stuff to that return that; }()); 

Now, of course, because the function is not called as a constructor with the new keyword or as a method, this bound to window , and they define that as this . Thus, they basically flush everything in the global object, and all of their submodule names are actually aliases for window . Is there a reason someone would want to do this? Or is it really as wrong as it seems to me?

Edit:

I was mistaken in putting var before the definition of the submodule, initially I wrote something a little different and forgot to delete var . I tried to make the example even clearer, I hope it will be more obvious what I mean now.

Edit 2:

I also looked at the scripts executed in Firebug, and they definitely add everything to the window , this object is a complete mess.

Answer 1

Yes, that doesn’t look right.

 MODULENAME = MODULENAME || {}; // missing var var MODULENAME.SUBMODULENAME = (function() { // probably the missing var from above... var that = this; //add some stuff to that return that; // that is the WINDOW- wrong. }()); 

DEMO for the damage he can do:

 var x = function() { alert('out'); } var MODULENAME = MODULENAME || {}; MODULENAME.SUBMODULENAME = (function() { var that = this; that.x = function() { alert('DAMAGE'); } }()); x();​ // alert DAMAGE and not "out" - messed up with the global object! 

Answer 2

The module template is used incorrectly, and one of the reasons why the function expression should not be used where their use gives nothing over the function declaration. If the goal is to create global functions (which I doubt it is), then they should use:

 function somefuncion() { ... } 

If their intention adds properties (in this case, methods) to the object, which is most likely the case, then:

 MODULENAME.SUBMODULENAME.somemethod = function() { /* do stuff */ }; 

If there is a need for conditional creation of methods, for example. Based on the detection of symptoms, the following may be required:

 (function(global, undefined) { // In here global is the global object global.MODULENAME = global.MODULENAME || {}; global.MODULENAME.SUBMODULENAME = global.MODULENAME.SUBMODULENAME || {}; // and undefined is undefined, belt and braces approach undefined = void 0; // Direct assignment function somemethod() { //do stuff }; // Assign directly to the "namespace" object MODULENAME.SUBMODULENAME.somemethod = somemethod; // Conditional assignment if ( sometest ) { MODULENAME.SUBMODULENAME.anothermethod = function(){...}; // Try another way... } else if (someOtherTest) { MODULENAME.SUBMODULENAME.anothermethod = function(){...}; // Default } else { MODULENAME.SUBMODULENAME.anothermethod = function(){...}; } // Clean up global = null; }(this)); 

One problem is that every function declared inside an external function has a short circuit back to the function object and its environment, so it is a bit wasteful for resources. It is much more efficient to keep it simple and use the module template where it is really needed, and just use the declarations and assignments of a simple function where it is not. Not so much fun, but more practical.