constructor inside constructor - bad practice?

Question

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(global_objects)
{
    this.field_processors =
    {
        "Phone1Number": new phone_processor(),
        "Phone2Number": new phone_processor(),
        "Phone3Number": new phone_processor()
    }
    //...here some other code

    /**
     * @constructor
     */
    function phone_processor()
    {
        //...here some other code

        function clear_phone(phone)
        {
            //...here some code
        }
        this.is_equals = function (value1, value2) 
        {
            return is_equals(clear_phone(value1), clear_phone(value2));
        }
    }
}

//... later in the code
var valid = this.field_processors[fld_name]["is_equals"](native_value, custom_value)

Do you think phone_processor constructor function should be outside contact_duplicates_manager?

This is merely a function within a function, and there's nothing wrong with it. Encapsulating code that is only used in the parent function is in my opinion a good practice. — mekwall
– mekwall, Commented Dec 26, 2013 at 11:57
@nnnnnn, I've updated the example to demonstrate how it's used. — Max Koretskyi
– Max Koretskyi, Commented Dec 26, 2013 at 11:59

Bergi · Accepted Answer · 2013-12-26 12:59:03Z

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:

function Item(…) {…}
function Store {
    var that = this;
    this.items = [];
    this.addItem = function(…) {
        that.items.push(new LocalItem(…));
    };
    function LocalItem(…) {
        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.

Collectives™ on Stack Overflow

constructor inside constructor - bad practice?

1 Answer 1

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Linked

Related