Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

constructor/initialize - suggestion #28

Open
gnimmelf opened this issue Aug 11, 2015 · 0 comments
Open

constructor/initialize - suggestion #28

gnimmelf opened this issue Aug 11, 2015 · 0 comments

Comments

@gnimmelf
Copy link

Hi again,

in ref to #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 =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant