tc39 / proposal-flatMap

proposal for flatten and flatMap on arrays
https://tc39.github.io/proposal-flatMap
214 stars 19 forks source link

Bikeshed: flatten(3) vs. flatten({ depth: 3 }) #33

Closed domenic closed 7 years ago

domenic commented 7 years ago

Probably the former is fine, but it might be worth discussing.

michaelficarra commented 7 years ago

I think the named parameters would be preferable if we were ever going to add a second parameter, but I really really really doubt we'd ever want to do that. As is, I'd be happy to get rid of the depth parameter.

bterlson commented 7 years ago

I want depth, but I am not aware of any other parameters to flatten APIs in other environs. Are there any?

domenic commented 7 years ago

I don't know of any and couldn't imagine any. It's more just a readability/style discussion, as to whether unnamed numeric parameters to an API that doesn't obviously deal in numbers are a good idea.

For example, I think if the parameter was a deep boolean, we'd put it in an options object, because flatten(true) doesn't obviously mean "flatten deeply".

Do similar considerations apply? Or do we think it's obvious enough that flatten(3) means "flatten to depth 3"?

I think it's probably obvious enough. But wanted to open an issue to check and discuss.

ljharb commented 7 years ago

I think it's obvious enough in this case, but I also think that I'd rather have the inconvenience of an options object arg that could have been a single value; over the inconvenience of having to add more args, or a second arg that's an options object, later if a use case pops up :-/

aluanhaddad commented 7 years ago

To my mind, this doesn't help the reader understand the things which may not be self-evident, such as how jagged depths are handled. That said, there's nothing wrong with being explicit here, and named arguments are awesome, but it would make the method an oddball among the existing array methods.

Honestly though, I'm so ecstatic that flatMap may actually get into the language, that I'll be happy with whatever parameter structure is ultimately decided upon.

michaelficarra commented 7 years ago

This method would stand out far too much from other built-in prototype methods. In private conversations with @bterlson, he agreed. Closing. Feel free to bring it up at the meeting tomorrow if you still think it's important.

domenic commented 7 years ago

I guess upon reviewing other built-ins, I do agree, e.g. in parseInt(x, r) the second parameter is pretty ambiguous, but we do not use an options object to help. Thanks for considering.

Jessidhia commented 7 years ago

Do we need to think of how to handle .flatten(Infinity)? That could be used to mean flatten deeply, but if you actually try that now, you get an infinite loop 😄

domenic commented 7 years ago

@Kovensky sounds unrelated to this issue, which BTW is already closed.

Jessidhia commented 7 years ago

Ah, was responding to the .flatten(true) comment. Should I open this as a separate issue?