AmpersandJS / ampersand-rest-collection

ampersand-collection with REST and lodash mixins for easy use with REST APIs.
http://ampersandjs.com/docs#ampersand-rest-collection
MIT License
23 stars 11 forks source link

constructor/initialize - suggestion #28

Open gnimmelf opened 9 years ago

gnimmelf commented 9 years ago

Hi again,

in ref to https://github.com/AmpersandJS/ampersand-rest-collection/issues/26, I just wanted to get seconds on this opinion:

IMHO, it doesn't make sense for the rest-collection to have the same constructor as the basic collection; most use-cases will have the data loaded from an endpoint, not passed in to the constructor. So making the models-array param optional makes sense.

So the rest-collection constructor could do

module.exports = function(models, options) {
    // Resolve optional arguments
    if (isObject(models) && options === undefined) {
        options = models;
        models = null;
    }

    // [...]

   // Pass optional arguments to `initialize`
   var params = [options];
   if (isArray(models)) { params.unshift(models) };
   this.initialize.apply(this, params);

   if (models) this.reset(models, assign({silent: true}, options));
}

making sure you get the same arguments in your extended rest-collection.initialize as you pass into the constructor.

Then again, maybe I'm just a tad bit too fanatical here =)