Constructor Inside Constructor - Bad Practice?

I'm exploring some code and I saw the practice of embedding one constructor function into another used many times: /** * @constructor */ function contact_duplicates_manager(globa

Solution 1:

Do you think phone_processor function constructor should be outside contact_duplicates_manager?

Yes. While being valid and working, it's not efficient and possibly unreadable. With the nesting, every contact_duplicates_manager instance has phone_processors with different constructors and inheriting from a different prototype object. This might be necessary for constructor factories or similar patterns, but those are very rare and I doubt you need it here.

My rules of thumb:

• Move every function that does not need access to any local closure variable to a higher scope.
• If you don't want it to be public in there, use an intermediate scope of an IEFE.
• If you need a constructor inside the scope of a multiple-times-executed function, try to share the prototype object among the constructors, and do not leak a local constructor.

An example for the last rule:

functionItem(…) {…}
functionStore {
    var that = this;
    this.items = [];
    this.addItem = function(…) {
        that.items.push(newLocalItem(…));
    };
    functionLocalItem(…) {
        Item.call(this, …);
        this.store = that;
    }
    LocalItem.prototype = Item.prototype;
}

You don't necessarily need the global Item function which you call inheritance-like, sometimes a single global proto object which you can assign to LocalConstructor.prototype is enough.