Javascript / NodeJS : Passed in object seems to overwrite "self" - closure issue?

Question

My first ever Stackoverflow question (that I can remember). I'm slowly getting back into development after many years absence and experimenting with Node and Javascript.

I'm hoping this is really simple but I just can't see what I'm doing wrong.

I have 2 instances of a fairly simple javascript object. When I pass one instance of that object to a function on the other things fall down.

• The first time I reference a "self" value it's fine.

• I then call a function on the passed-in object.

• If I try to reference "self" again afterward, it seems that "self" has been overwritten by the passed-in object.

Here's the object source:

"use strict";
//simple object
module.exports.SimpleObject = function SimpleObject(inputName) {
    try{
        var self = this; //closure so we don't lose this reference in callbacks
        self.name = inputName; 

    }
    catch(err) {
        console.log('Unable to create SimpleObject: '+err);
    }

    SimpleObject.prototype.getName = function() {
        self = this;
        return self.name;
    }

    SimpleObject.prototype.nestedOverwrite = function(anObject) {
        self = this;
        return "#1 self.name: "+self.name+" ObjectName: "+anObject.getName()+" #2 self.name: "+self.name;
    }

    return this;
}

---

And here's the test case:

var  testSimpleObjectModule = require('./simpleObject');

var o0 = new testSimpleObjectModule.SimpleObject('test0');
var o1 = new testSimpleObjectModule.SimpleObject('test1');
console.log(o0.nestedOverwrite(o1));
console.log(o1.nestedOverwrite(o0));

---

Here's the output I get:

#1 self.name: test0 ObjectName: test1 #2 self.name: test1
#1 self.name: test1 ObjectName: test0 #2 self.name: test0

Hopefully you can see in each case, the first call to self.name is fine, the second has lost context.

any help greatly appreciated!

Answer 1

You're assigning functions to the prototype object that gets assigned to new SimpleObject every time the constructor is called. So the second time the constructor is called, the prototype is updated, and any previous object created using the prototype starts using the new function (with the new self value).

If you want those closures to have self, don't put them on the prototype, put them on this (more below).

But note that in the code you've quoted, there's no reason for the self variable. You can use prototype functions (which is more efficient) rather than creating functions for each instance. To do that, use this (without assigning it to self) and define the functions outside the constructor, like this:

"use strict";
//simple object
module.exports.SimpleObject = function SimpleObject(inputName) {
    this.name = inputName;
};

SimpleObject.prototype.getName = function() {
    return this.name;
};

SimpleObject.prototype.nestedOverwrite = function(anObject) {
    return "#1 this.name: "+this.name+" ObjectName: "+anObject.getName()+" #2 this.name: "+this.name;
};

But if you're doing something with self that you aren't showing that means you really need it (e.g., that you can't trust the value of this when the functions are called &mdasdh; they're being used as callbacks/event handlers, etc.), here's how you do that:

"use strict";
//simple object
module.exports.SimpleObject = function SimpleObject(inputName) {
    var self = this;

    self.name = inputName; 

    self.getName = function() {
        return self.name;
    };

    self.nestedOverwrite = function(anObject) {
        return "#1 self.name: "+self.name+" ObjectName: "+anObject.getName()+" #2 self.name: "+self.name;
    };
};

---

Side notes:

• There's no reason to return this at the end of a constructor function.

• Note that we don't want to do self = this; in each of the functions; the whole point of using self is that when the function is called, you don't want to rely on this having the correct value. (If yoU can rely on this being set, then use the prototype version of the functions.)

• Never catch an exception in a constructor function and just say you couldn't do the construction, because by doing that, you allow the new Xyz expression to complete, and the un-initialized object is returned as the result of the expression. I've removed the try/catch from the above for that reason.

• I recommend not relying on the horror that is automatic semicolon insertion. End expressions with semicolons (including when you're assigning a function to a variable or property).