Add index param to when.map() mapFunc #134

Closed
briancavalier opened this Issue Mar 26, 2013 · 12 comments

Comments

Projects
None yet
8 participants
Owner

briancavalier commented Mar 26, 2013

Currently, we call mapFunc like: mapFunc(x). This is different Array.prototype.map. Also, we do supply the index to when.reduce's reduceFunc. It's easy to add, but sometimes can be annoying in cases where you want to use an existing function that might do weird things if passed a second param.

My current thinking is that we wait until someone asks for it.

Owner

unscriptable commented Mar 26, 2013

... or plan to change it in a major release? Inconsistency amongst these iterator functions seems more annoying, imho. See jQuery.each(). :)

Owner

briancavalier commented Mar 26, 2013

Yeah, you're probably right about inconsistency being more annoying. I'll schedule it for 3.0. If we find a reason not to add it in the meantime, we can just close this issue.

Owner

scothis commented Mar 26, 2013

Better alignment with Array.proto.map makes sense, 2.1 seems like a good time. No need to wait for 3.0

Owner

briancavalier commented Mar 26, 2013

I'm torn.

There's nothing technically "broken" about it now, and no one is complaining. Plus, we might actually break someone's code by introducing it. One could argue (rightfully, imho!) that any code that relies on a missing 2nd param simply is wrong, but nonetheless, it may break someone's app.

Maybe we should just ask people, via gg and twitter, to weigh in.

I would also be interested in the signature of the mapFunc in keys/map providing the current key as the second argument, similar to behavior in underscore's map.

Owner

briancavalier commented Sep 3, 2013

@tgriesser That makes perfect sense, thanks for the suggestion!

acjay commented Oct 24, 2013

@briancavalier I just ran into the missing index issue, so I would greatly appreciate having it :)

For functions that have additional parameters, they should probably be wrapped in a function expression that discards the extra params from map. But I think consistency with Array.prototype.map is highly desired.

Owner

briancavalier commented Oct 24, 2013

Thanks for chiming in, @acjay. Now that we have a few folks in favor of this, and no one really against it, I think we should schedule it for 3.0.

acjay commented Oct 24, 2013

Awesome!

phated commented May 7, 2014

Talking about this with @briancavalier on IRC. I had expected it to, but seeing the implementation, I get why it doesn't. It would be a nice-to-have for me.

@briancavalier briancavalier removed this from the 3.0.0 milestone May 8, 2014

adelura commented Jul 9, 2014

Do you still have in mind adding key/index as a second argument for mapper function in when.map function for objects/arrays? @briancavalier you scheduled it for version 3, but current one is 3.3.1. I think it's usefull feature and there are common use cases just like in native Array.prototype.map method.

Contributor

jescalan commented Jul 14, 2014

+1 for keys as a second param!

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