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

Add example of destructuring assignment in parameter list to documentation #2663

Closed
wants to merge 2 commits into from

Conversation

kevinrobinson
Copy link

CoffeeScript is amazing. I just found out that it supports destructuring assignment even inside the parameter list of a function. It's a great feature and so I thought it'd be good to call it out in the documentation.

It looks like there was some discussion about this in class constructors in #1607 and #2378 and it just needed to get added. So here's a pull request for documenting its use in a plain function, rather than the more complicated of a class method.

Thanks!

@vendethiel
Copy link
Collaborator

You shouldn't rebuild the docs, they'll be rebuilt by jashkenas when releasing

@kevinrobinson
Copy link
Author

@Nami-Doc Ahh, thanks. I see that in the contributing notes now, my mistake.

@jashkenas
Copy link
Owner

Quite right -- can you remove the re-built docs from this pull request?

@kevinrobinson
Copy link
Author

Okay, I think this is all set now, sorry about that!

@jashkenas
Copy link
Owner

You know -- I'm actually a bit reluctant to merge this example code. Despite destructuring being generally valid on any LHS in CoffeeScript, it's isn't "best practice" to use it in an argument list. I think it makes it significantly harder to read what arguments the function expects. So I'd rather leave it out of the official documentation, so as not to encourage the pattern.

@jashkenas jashkenas closed this Mar 4, 2013
@kevinrobinson
Copy link
Author

Jeremy, thanks for the thoughtful response as always.

In practice I actually use it much more with arrays representing tuples rather than objects, so maybe that was a poor example in the pull request. Curious if you'd describe this as an anti-pattern as well.

Given:

namePointTuples = [
  ['kevin', 432]
  ['jeremy', 235]
  ['jon', 324]
]

You'd prefer:

messages = namePointTuples.map (tuple) ->
  [name, points] = tuple
  "#{name}: #{points}pts"

over this:

messages = namePointTuples.map ([name, points]) ->
  "#{name}: #{points}pts"

is that right?

The reason that the destructuring inside the argument list feels like a good idea to me is that if there's a structure required of a parameter, it makes that explicit in the params list for the function (rather than lower down in the function body). In my example here I was clear and did the destructuring on the first line (rather than in the string interpolation), but it seems like a natural extension to me to just push that directly into the parameters list.

Anyway, thanks for the feedback!

@jashkenas
Copy link
Owner

is that right?

That's right! It makes it easier to read at a glance that the function expects a tuple as the first argument. In the second example, you can still see what's going on, but it's a little harder to parse (for me). As always, let your own personal readability preference decide.

@kevinrobinson
Copy link
Author

Thanks again for the thoughts (and the language)!

@davidchambers
Copy link
Contributor

Despite destructuring being generally valid on any LHS in CoffeeScript, it's isn't "best practice" to use it in an argument list. I think it makes it significantly harder to read what arguments the function expects.

In general, @jashkenas, I agree, though the reverse is true in cases where the parameter is difficult to name:

get = (url, callbacks) ->

What is callbacks? An array of functions? An object of some sort?

get = (url, {error, success, complete}) ->

Oh, I see, it's an object with members named "error", "success", and "complete".

@vendethiel
Copy link
Collaborator

But then, what are they ;)? Callbacks ? Count of each case? Flags ?

@davidchambers
Copy link
Contributor

Good point, @Nami-Doc. Naming things is hard!

@vendethiel
Copy link
Collaborator

Note that as-pattern would solve the problem, but again adds complexity to paramlist :(.

@michaelficarra
Copy link
Collaborator

Yep, as-patterns are the real solution here.

@kevinrobinson
Copy link
Author

@michaelficarra and @Nami-Doc, I'm curious about as-patterns and can find they're in Haskell. Is there an open issue or proposal to bring this kind of pattern matching to CoffeeScript?

Seems to me like the closest we could get today would be this below (really just trying to understand here).

In Haskell, for modeling a conditional addition operation (a terrible example for sure):

conditionalSum t@(bool,x,y) = if bool
                 then show(x + y)
                 else "(not added)"

In CoffeeScript:

# utility function
asPattern = (body) -> (param) -> body param, param

# using the utility function
conditionalSum = asPattern (t, [bool, x, y]) -> if bool then "#{x + y}" else "(not added)"

It just passes the argument twice so that it can be destructured the second time. This doesn't seem super useful to me, just checking if understand your idea. :)

@vendethiel
Copy link
Collaborator

get = (url, {sucess, error}:callbacks) ->
#2475

@kevinrobinson
Copy link
Author

@Nami-Doc Thanks!

@maxtaco
Copy link
Contributor

maxtaco commented Sep 14, 2013

I'm late to this conversation, but I think foo = ({x,y}) -> x+y is a great feature, since it frees the user of the API from having to remember which order arguments are expected. Python has this feature, and it's great, especially for functions (or methods) with long argument lists. It's annoying to hunt down bugs caused by transpositions in positional arguments, and they're especially common in a language like CS without static type-checking.

I use this feature all of the time, but only today realized that it's obscure and undocumented because, ironically, I was trying to document my own code with codo which doesn't support this feature.

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

Successfully merging this pull request may close these issues.

6 participants