Javascript - Is it a bad idea to use function constructors within closures?

Question

I'm wondering if there are any memory or performance issues when using function constructors within a closure?

Here is a crude example, I know there are a tonne of different and better ways to write this, I just wanted to provide an example where each constructor now has access to variable in the closure (farmer,lastToSpeak and the animals).

// usage: myFarm = new Farm(["Cow","Goat"],"Old MacDonald");

function Farm(animalList, farmer){
    var animals = [],
        lastToSpeak = "";

    function Cow(){
        this.speak = function(){
            alert("I'm a Cow and I belong to "+farmer);
        }
        lastToSpeak = "A Cow";
    }
    function Sheep(){
        this.speak = function(){
            alert("I'm a Sheep and I belong to "+farmer);
        }
        lastToSpeak = "A Sheep";
    }
    function Goat(){
        this.speak = function(){
            alert("I'm a Goat and I belong to "+farmer);
        }
        lastToSpeak = "A Goat";
    }

    for(var i = 0; i < animalList.length; i++){
        switch(animalList[i]){
            case "Cow":
                animals.push(new Cow());
                break;
            case "Sheep":
                animals.push(new Sheep());
                break;
            case "Goat":
                animals.push(new Goat());
                break;
        }
    }

    this.allSpeak = function(){
        for(var i = 0; i < animals.length; i++){
            animals[i].speak();
        }
    }
}
myFarm = new Farm(["Cow","Goat"],"Old MacDonald");
myFarm.allSpeak();

I'm guessing in this example it makes little difference, but if the cow,sheep and goat were much larger and complex and we were creating lots of farms would there be any disadvantages to this approach?

Will a new set of constructor functions be stored to memory every time a farm is created?

UPDATE

So i'm happy with what Kemal Dağ said and also the comment from Bergi.

If I changed the code to use prototypes and passed in the farm as Bergi suggests, does that seem like the better approach?

function Farm(animalList, farmer){
    var animals = [],
        lastToSpeak = "";

    this.farmer = farmer;

    for(var i = 0; i < animalList.length; i++){
        switch(animalList[i]){
            case "Cow":
                animals.push(new this.Cow(this));
                break;
            case "Sheep":
                animals.push(new this.Sheep(this));
                break;
            case "Goat":
                animals.push(new this.Goat(this));
                break;
        }
    }

    this.allSpeak = function(){
        for(var i = 0; i < animals.length; i++){
            animals[i].speak();
        }
    }
}
Farm.prototype.Goat = function(farm){
    this.speak = function(){
        alert("I'm a Goat and I belong to "+farm.farmer);
    }
    farm.lastToSpeak = "A Goat";
}
Farm.prototype.Cow = function(farm){
    this.speak = function(){
        alert("I'm a Cow and I belong to "+farm.farmer);
    }
    farm.lastToSpeak = "A Cow";
}
Farm.prototype.Sheep = function(farm){
    this.speak = function(){
        alert("I'm a Sheep and I belong to "+farm.farmer);
    }
    farm.lastToSpeak = "A Sheep";
}

myFarm = new Farm(["Cow","Goat"],"Old MacDonald");
myFarm.allSpeak();

UPDATE

I put together a fiddle rather than add another version to the question here. I've separated my animal constructors completely and moved speakAll() to the prototype. I think I was really looking for a solution that allowed me to share variables across my instances without adding anything to the global scope. I finally decided to pass an object to each instance rather than the constructor, meaning I don't have to make them public on the constructor. Thanks Guys.

If every of these constructors really needs access to the closure variables (like farmer in your case) then it's fine. Putting them outside and passing them the farm instance as an argument might be more convenient though. — Bergi
– Bergi, Commented Sep 12, 2013 at 10:49
Your latest update would just work fine without any performance issues, but you can also define Goat, Cow and Sheep functions as independent objects, and inject them into Farm object. They totally does not need to be defined in related to Farm object unless you want them to be private. — Kemal Dağ
– Kemal Dağ, Commented Sep 12, 2013 at 11:01

Kemal Dağ · Accepted Answer · 2013-09-12 10:58:04Z

In javascript every function itself is an object, so the answer is yes to your question. Every time you create a Farm object you also create its all private methods in your case they are : Cow, Sheep, Goat.

To prevent this, create functions in prototype of Farm object. This prevents re-allocation of memory for those functions, but they immediately become public functions. See:

Farm.prototype.Goat = function(farmer){
        this.speak = function(){
            alert("I'm a Goat and I belong to "+farmer);
        }
        this.lastToSpeak = "A Goat";
}

You have to decide that private functions are vital for you or not? Then prototypal approach is much better.

But remember, using prototypes is a bit tricky in javascript, so I had to modify some aspect of your code, you can see a working example here.

In that example, you'll find that this.lastToSpeak assignment is fundementally wrong. Since you are using function created at prototype of Farm object as a Object constructor itself. Then this becomes reference to Goat object not Farm object, So if you want to refer parent Farm object you probabily pass a reference to parent to Goat constructor. Remember, prototypal inheritence is nothing like standart class inheritence, but you can pretty much emulate any behaviour you want.

Bergi · Accepted Answer · 2013-09-12 11:14:23Z

2

Will a new set of constructor functions be stored to memory every time a farm is created?

Yes. However, if they really need access to the closure variables (like lastToSpeak) that can be reasonable, just as it is for privileged methods. Having private constructors might be a bit odd, but can be required.

When you are dealing with a set of similar (but differently-scoped) constructors it can be a performance optimisation to give them a common prototype object. If the prototype methods do not need access to the closure variables as well, then use a static object outside the Farm constructor and assign that to each privileged constructor.

I changed the code to use prototypes, does that seem like the better approach?

Not like this. Constructors should not be instance methods, putting them on the prototype object is odd. Better put them on the Farm function object as a namespace.

Here's an example of making everything public, worshipping prototypes:

function Farm(animalList, farmer){
    this.farmer = farmer;
    this.lastToSpeak = "";
    this.animals = [];

    for(var i = 0; i < animalList.length; i++){
        var name = animalList[i];
        if (Farm.hasOwnProperty(name))
            this.animals.push(new Farm[name](this));
    }
}
Farm.prototype.allSpeak = function(){
    for(var i = 0; i < this.animals.length; i++){
        this.animals[i].speak();
    }
};

function Animal(farm) {
    this.farm = farm;
    farm.lastToSpeak = "A "+this.type;
}
Animal.prototype.speak = function(){
    alert("I'm a "+this.type+" and I belong to "+this.farm.farmer);
};
Animal.create = function(name) {
    Farm[name] = function() {
        Animal.apply(this, arguments);
    };
    Farm[name].prototype = Object.create(Animal.prototype);
    Farm[name].prototype.type = name;
};
Animal.create("Goat");
Animal.create("Sheep");
Animal.create("Cow");

myFarm = new Farm(["Cow","Goat"],"Old MacDonald");
myFarm.allSpeak();

edited Sep 12, 2013 at 11:14

answered Sep 12, 2013 at 11:02

Bergi

671k162 gold badges1k silver badges1.5k bronze badges

2 Comments

Ben Over a year ago

Thanks, when you say "Better put them on the Farm function object as a namespace", do you mean changing Farm.prototype.Sheep to just Farm.Sheep and calling new Farm.Sheep as opposed to new this.Sheep? I tried that and it worked, just wanted to check I understand.

Ben Over a year ago

@Bergi Sorry, I posted my last update before looking at this. Problem for me was that it took me about 10mins to understand everything here :), after that I can definitely see that its better optimized and quite elegant. Really appreciate it, Thanks.

Collectives™ on Stack Overflow

Javascript - Is it a bad idea to use function constructors within closures?

2 Answers 2

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related