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

Misleading documentation re overriding Model.set #1391

Closed
molecularbear opened this issue Jun 8, 2012 · 3 comments
Closed

Misleading documentation re overriding Model.set #1391

molecularbear opened this issue Jun 8, 2012 · 3 comments

Comments

@molecularbear
Copy link

The documentation defines Model.set's function arguments like this:

model.set(attributes, [options])

And down further states that one could override set like this:

var Note = Backbone.Model.extend({
    set: function(attributes, options) {
        Backbone.Model.prototype.set.call(this, attributes, options);
        ...
    }
});

But this will result in buggy code because the real function arguments from the actual code are:

set: function(key, value, options) {

So in order to properly override set, one would need specify the true arguments, and juggle those arguments around to determine whether value is really a value, or whether value is actually options, and so forth. It might also be a good idea to mention that unset calls set.

@wookiehangover
Copy link
Collaborator

I think that the assumption is that if you're overwriting provided methods that you're more or less going to be in control of how you're going to use them, and therefore would know if you needed to account flexible arity situations.

FWIW, there's also an even easier way to preserve this if you're ultimately falling back to the original method: just use apply instead of call

Backbone.Model.prototype.set.apply(this, arguments);

@braddunbar
Copy link
Collaborator

I agree with @wookiehangover. Using apply instead is a much nicer way to ensure the original arguments get passed (just make sure you don't change them).

As for unset calling set, that's an implementation detail that can be garnered from reading the source and isn't really relevant to the documentation.

Thanks for pointing this out @molecularbear!

@wookiehangover
Copy link
Collaborator

TIL what variadic means. Thanks @braddunbar!

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

3 participants