javascriptdesign-patternsglobal-object

this keyword in module pattern?


I've just started working at a new company and noticed something that looks completely wrong to me in a lot of their JS. I'm a bit hesitant to bring it up without confirming this is wrong since I'm pretty junior, I'm not a JS expert and it's only my second day and I don't want to look stupid.

So, normally I'd expect the module pattern to look something like:

MODULENAME = MODULENAME || {};

MODULENAME.SUBMODULENAME = (function() {
    var bla = {};

    bla.somefunction = function() {
        //do stuff
    };

    //add more stuff to bla
    return bla;
}());

What they have all over their code is:

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 isn't being called as a constructor with the new keyword or as a method, this is bound to window and they're defining that as this. So they're basically dumping everything in the global object and all their sub-module names are in fact aliases for window. Is there any reason anyone would want to do this? Or is this really as wrong as it seems to me?

Edit:

I made a mistake in putting var before the submodule definition, originally I wrote something slightly different and forgot to delete the var. I've tried to make the example a bit clearer too, hopefully it's more obvious what I mean now.

Edit 2:

Also I've looked at the scripts executing in Firebug and they are definitely adding everything to window, that object is a total mess.


Solution

  • Yes it looks wrong.

    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 it 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!