-
Notifications
You must be signed in to change notification settings - Fork 2k
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
destructuring parameters currently undocumented #1607
Comments
whoa nice. never knew about this, looks quite useful! :P |
Reminds me of ooc :D
Feels good ^^. |
Huh, I thoought this was documented because I knew about it. I wonder how I figured it out. |
@erisdiscord Probably because it's shown in the Class example. |
@codelahoma is it? where? |
Note I said shown, not documented. :-) The Animal class here has a bodyless constructor: http://coffeescript.org/#classes |
@codelahoma that's different, this issue is about passing named params to constructor. read closely. |
@secoif Gotcha (that IS neat). Sorry about the noise. |
It actually works for functions in general, not just constructors. fn = ({a,b}) ->
console.log "a is #{a}, b is #{b}"
fn
b: 5
a: 1 compiles to var fn;
fn = function(_arg) {
var a, b;
a = _arg.a, b = _arg.b;
return console.log("a is " + a + ", b is " + b);
};
fn({
b: 5,
a: 1
}); @jashkenas Would it be better to document this in the Destructuring Assignment section, or in the Function Literals section with a link to the DA section? |
Honestly, I forgot that we supported that ... and I'm not a huge fan of it, because it seems to make your function's API a bit less documented than writing it out more clearly:
... so that's probably the original reason why it's not documented. |
Is this whole issue a non-starter, then? Or is it just the plain function application you don't like, and the constructor usage should be documented? |
I don't know. If we do document it, we should find a good real-world example where it really helps make the code more readable. |
@jashkenas: It eliminates a temporary variable. Any example of its usage would be a good example. I don't see why you don't like the feature. You say the function's API looks less documented, but it's definitely more clear to me when the possible keys for an options argument are listed right in the parameter list. |
Yeah I don't see how this: fn = (options) ->
{a, b} = options is less documented than this: fn = ({@a, @b}) ->
…because it's at least as documented as this: fn = (@a, @b) ->
…which is already acceptable? |
Depending on how you look at it, it also either shadows globals unexpectedly or prevents accidental modification of globals: http://bit.ly/vuKchh I agree with @michaelficarra that the destructuring syntax is more clear, not less. It not only lists the possible keys, but also lets you know at a glance that the parameter is an object. |
I sneaked it in along with the default argument support.
Understandable, considering you weren't a fan of default arguments either. |
@jashkenas I'm happy to put some energy into adding to the already excellent documentation. Are you OK to accept the documentation patch on this if I work out an example that you think is clear and relevant? |
Sure -- clear, relevant, and useful documentation patches are always welcome. The trick with documenting this one will be in finding a case where the feature is truly readable and useful. |
@jashkenas I think the argument here is that using the object literal variable expansion syntax inside the arguments of a class's constructor is just a shorthand way of initializing instance properties to keys passed in an object to the constructor. Here's my example, elaborated on @timoxley's original code above... class PersonA
constructor: ({@name, @likes}) ->
# The obect literal expansion syntax above
# auto-initializes properties on this instance
# to the matching keys on the object passed to the constructor.
person = new PersonA name: 'Homer', likes: 'Mindy'
console.log "#{person.name} likes #{person.likes}"
# "Homer likes Mindy"
# Versus....
class PersonB
constructor: (options) ->
# Here, we're manually assigning properties
# on this instance to the matching keys
# on the options object passed to the constructor.
@name = options.name
@description = options.description
person = new PersonB name: 'Homer', likes: 'Mindy'
console.log "#{person.name} likes #{person.likes}"
# "Homer likes Mindy"
# We get the same functionality, but PersonA's constructor is more succinct. Is this clear enough? I'm happy to try a different example if needed. Otherwise, let me know if you're ok with this and I'll get a pull request in. |
It's clear, but it's lengthy -- it would be spending a significant amount of reader time and space on this feature, on the homepage, when it's not necessarily something we'd like to encourage folks to write in the first place. Given your example, I think this would be preferred:
A good place for a longer description of all this would be on the Wiki, I'd imagine. |
@jashkenas sorry, but can you elaborate on why you prefer two-liner over the single line option? |
Sure -- it's clearer about what the signature of the method is, and is easier to read. Where the former would require an explanation as to what is going on, the latter is fairly comprehensible if you know how destructuring assignment works. |
I guess you could argue that the other way too: i.e. it's clearer about what the signature of the method is, the former is fairly comprehensible if you know how destructuring parameters works. edit: but there is the arguable cost of "yet another syntax"… |
@timoxley , I agree with @jashkenas in this case. I'd propose adding one more section to the destructuring asignment section referencing how you can leverage it in the common case of a class constructor taking arguments (per Jeremy's example above). @jashkenas you good with this? If so, I'll throw a pull request together to save you the time. |
Sure. |
The discussion continues in @gabehollombe's pull request: #2378 |
The destructuring parameter lists syntax isn't mentioned in the docs anywhere, and the syntax is really neat:
Recommend adding to documentation.
The text was updated successfully, but these errors were encountered: