Javascript - Is It A Bad Idea To Use Function Constructors Within Closures?

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 be

Solution 1:

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.

Solution 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:

functionFarm(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(newFarm[name](this));
    }
}
Farm.prototype.allSpeak = function(){
    for(var i = 0; i < this.animals.length; i++){
        this.animals[i].speak();
    }
};

functionAnimal(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 = newFarm(["Cow","Goat"],"Old MacDonald");
myFarm.allSpeak();