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.

Answer 1

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();

Answer 2

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.