Why these closure based private variables in JavaScript not working

Question

I am experimenting imitating OOP like behavior in JS. I am trying to have (private) variables: id and name in function Person. To this function I am passing arguments which are used to initialize (private) variables. Then I am returning object having getter and setter for name and only a getter for id, thus effectively making id read-only.

So id can be set only through constructor whereas name can be set and get anytime.

This is the code:

     var Person = function (_id,_nm) {
        var id, name;    
                  
        this.id = _id;
        this.name = _nm;

        return {               
            setName: function (nm) {
                name = nm;
            },
            getName: function () {
                return name;
            },
            getId: function () {
                return id;
            },
            print: function () {
                document.writeln("Id: "+id+"<br />Name: "+name);
            }
        }
    }

    var person = new Person(123, "Mahesh");
    person.print();

However when new Person(123,"Mahesh") executes, I dont understand it is actually setting id and name or not, since while debugging I can see values set appropriately when hovered over them but Locals panel does not show them initialized:

Or either while in print() is is not referring to the desired id and name variables:

Whats wrong here?

Answer 1

Working fiddle

The reason is because you are using public members for the Person.prototype. You don't need to add this references to these two. So delete:

this.id = _id;
this.name = _nm;

and simply use:

 var id = _id,
     name = _nm;  

Now everything will work fine. The whole idea is to use var, and not this, otherwise a closure will not be created. Now you will not be able to access name and id directly, instead you will have to use setName(), getName(), setId(), getId() etc.

The two members, id and name, will now become closures as you want them to be.

Update

If you used this.id, then it wouldn't have been private and you could just do var p = new Person(1, "Mahesha"); and access p.name or p.id directly. They are supposed to be private so this is not what you want.

With the closure pattern, p.name and p.id are undefined and can only be accessed through p.getName(); and p.getId();. Read on how closures work. The idea is that because you are using that var name, a closure will be created to remember it's value.

Your getName and setName are using that closure to access the name property. There is no this.name, there is a value remembered through a higher - order closure.

Answer 2

You've mixed up using locally scoped ("private") variables for _id and _nm and "public" instance properties (this.id and this.nm).

In this case you need the former, but you created both and only initialised the latter.

Note that since id is read-only you don't really need a separate local variable at all, you can just use the lexically scoped first parameter to the constructor:

var Person = function (id, _nm) {
    var name = _nm;

    ...
};

Answer 3

this.id and var id are not the same. this.id is a property of the object. var id belongs to the local scope.

Answer 4

Either use new or return a value. Not both.

The problem is that you're creating a new instance of Person using the new keyword, but your constructor function is returning another object instead.

When you return something from a constructor function it returns that value, and not the instance of the function.

You see when you execute new Person(123, "Mahesh") a new instance of Person is created. This is accessible within the constructor function as this.

If you don't return anything from your constructor then JavaScript automatically returns this. However you're explicitly returning another object.

No wonder var person doesn't have id and name properties (which you only defined on this).

In addition print doesn't display the id and name because although you declared them (var id, name) you didn't give them any values. Hence they are undefined.

This is how I would rewrite your Person constructor:

function Person(id, name) {
    this.getId = function () {
        return id;
    };

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

    this.setName = function (new_name) {
        name = new_name;
    };

    this.print = function () {
        document.writeln("Id: " + id + "<br/>Name: " + name);
    };
}

I didn't set the id and name properties on this because it makes no sense to include them.

Answer 5

Let me try to explain using the following:

// Scope 1
var Person = function (_id,_nm) {
    // Scope 2
    var id, name;    

    this.id = _id;
    this.name = _nm;

    return {
        // Scope 3
        setName: function (nm) {
            name = nm;
        },
        getName: function () {
            return name;
        },
        getId: function () {
            return id;
        },
        print: function () {
            $("#output").append("<p>Id: "+id+"; Name: "+name  + "<p/>");
        }
    }
}

The print method will return undefined because you are referring to the var id, name; that is never set in your code. You set the _id and _name to the property id and name but you fail to return the object that you just created. Instead, you return a dictionary, that references the name and id variable you created in Scope 2 that you never set, hence the undefined output in the print() call.

Here is what you should have done:

var NewPerson = function (_id,_nm) {
    var self = this;
    this.id = _id;
    this.name = _nm;  

    this.print = function () {
        $("#output").append("<p>New Person Id: "+this.id+"; Name: "+ this.name  + "<p/>");
    };

    return this;
};

var nperson = new NewPerson(456, "Marcos");
nperson.print();

This will output:

New Person Id: 456; Name: Marcos

In essence, new is creating a new object represented by this, and you must return this to have a reference to the object created. In fact, if you do not use the new before creating an object, you end up referencing a global instance of this. Try the following:

var nperson = new NewPerson(456, "Marcos");
this.name = "Hello";
nperson.print();

var nperson1 = NewPerson(456, "Marcos");
this.name = "Hello";
nperson1.print();

You will see that the first output will be as expected:

New Person Id: 456; Name: Marcos

But the second will pick up the change made to this.name:

New Person Id: 456; Name: Hello

Here is the JSFiddle.