CODEWITHSUNDEEP
javascriptobjectdesign-patterns
inthenameofmusik
inthenameofmusik

Reputation: 623

Beginner JavaScript Pattern

I'm recently started studying objects, closures, scopes and the such. I'm trying to implement my own code using these techniques but am running into issues. 

function Person(name, age) {
    this.name = name;
    this.age  = age;

    return {
        ageUp: function ageUp() {
            this.age++;
        },
        printInfo: function printInfo() {
            console.log(this.name + " is " + this.age + " years old");
        },
        changeName: function changeName(newName) {
            this.name = newName;
        }
    }
}


var jeff = new Person('jeff', 28);
jeff.printInfo();

Oddly, this returns undefined is undefined years old. Does the printInfo property not have this.name in its scope because the returned object has no recollection of the function?

Also oddly enough, if I change all the instances of this.name and this.age to regular private variables (such as var personName = name;), somehow the returned object will operate properly and I can use the ageUp and printInfo as expected.

Am I combining two design patterns that I shouldn't be? What would be the best way to go about this?

Upvotes: 3

Views: 280

Answers (4)

Paul
Paul

Reputation: 141839

The problem is that when you do:

return { ...

You are creating a new object, separate from the object that the new keyword previously created (the one you assigned two properties to). You might notice that jeff instanceof Person is false and jeff.constructor === Object.

See this example:

function Person(name, age) {
    this.name = name;
    this.age  = age;

    return {
        theObjectThatNewCreated: this
    }
}

var jeff = new Person('jeff', 28);
console.log( JSON.stringify( jeff ) );
// {"theObjectThatNewCreated":{"name":"jeff","age":28}}
console.log( jeff.constructor );
// Object
console.log( jeff.theObjectThatNewCreated.constructor );
// Person

You could fix it by assigning the name and age properties to the object you return instead of to this:

function Person(name, age) {
    return {
        name: name,
        age: age,
        ageUp: function ageUp() {
            this.age++;
        },
        printInfo: function printInfo() {
            console.log(this.name + " is " + this.age + " years old");
        },
        changeName: function changeName(newName) {
            this.name = newName;
        }
    }
}


var jeff = new Person('jeff', 28);
jeff.printInfo();

But then Person isn't really a constructor, it is just an object factory and there is no point calling it with new. The objects it returns are not instances of Person, they are plain old Objects. There is a better way.

Am I combining two design patterns that I shouldn't be? What would be the best way to go about this?

I would say you are combining the revealing module pattern with a normal JavaScript constructor.

Instead of returning a new object you can just use this all the way through, to assign those functions as properties of the Person object instead of a new object:

function Person(name, age) {
    this.name = name;
    this.age  = age;

    this.ageUp = function ageUp() {
        this.age++;
    };

    this.printInfo = function printInfo() {
        console.log(this.name + " is " + this.age + " years old");
    };

    this.changeName = function changeName(newName) {
        this.name = newName;
    };
}


var jeff = new Person('jeff', 28);
jeff.printInfo();

But since those functions don't use any closed over variables from the constructor, they really should be added to the prototype of the constructor instead:

function Person(name, age) {
    this.name = name;
    this.age  = age;
}

Person.prototype.ageUp = function ageUp() {
    this.age++;
};

Person.prototype.printInfo = function printInfo() {
    console.log(this.name + " is " + this.age + " years old");
};

Person.prototype.changeName = function changeName(newName) {
    this.name = newName;
};

var jeff = new Person('jeff', 28);
jeff.printInfo();

Upvotes: 3

micha149
micha149

Reputation: 831

You are returning a new object in your constructor. So, you would not receive an instance of Person when calling new Person(...) rather than a dump object which has your defined properties.

Try to define your methods object as the prototype of your constructor:

function Person(name, age) {
    this.name = name;
    this.age  = age;
}

Person.prototype = {
    ageUp: function ageUp() {
        this.age++;
    },
    printInfo: function printInfo() {
        console.log(this.name + " is " + this.age + " years old");
    },
    changeName: function changeName(newName) {
        this.name = newName;
    }
}

Alternatively, if you want to keep your properties name and age private, your could define your methods in the constructor. Simply assign methods to this.. For private properties, you don't have to assign the properties itself to your instance:

function Person(name, age) {
    this.ageUp = function ageUp() {
        age++;
    };

    this.printInfo = function printInfo() {
        console.log(name + " is " + age + " years old");
    };

    this.changeName = function changeName(newName) {
        name = newName;
    };
}

But I would prefer the first example. In the second, each new instance will create new methods. In the first example, all your instances refers to a single prototype object which should use less memory.

Upvotes: 2

theharls
theharls

Reputation: 163

You are correct in that you are combining two patterns. Firstly, under normal circumstances, a constructor wouldn't return anything. (There are exceptions which I won't delve into here).

You probably want to do something like this:

function Person(name, age) {
    this.name = name;
    this.age  = age;
}

Person.prototype.printInfo = function() {
    console.log(this.name + " is " + this.age + " years old");
};

// and so on. Now you can say

var jeff = new Person('jeff', 28);
jeff.printInfo(); // => jeff is 28 years old

I am assuming you want those methods on the prototype so they are shared (it's not mandatory however).

Upvotes: 2

ViRuSTriNiTy
ViRuSTriNiTy

Reputation: 5155

Remove the surrounding return {} in your class and it should work.

Upvotes: 0

Related Questions