this keyword in module pattern?

Question

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 globa…

Accepted Answer

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

this keyword in module pattern?

Solution

Problem