Skip to main content

Bug Tracker

Side navigation

#2616 closed enhancement (fixed)

Opened March 29, 2008 05:27PM UTC

Closed April 10, 2011 08:46PM UTC

Last modified March 09, 2012 02:09PM UTC

A better jQuery.map

Reported by: flesler Owned by: danheberden
Priority: high Milestone: 1.6
Component: core Version: 1.5
Keywords: Cc:
Blocked by: Blocking:
Description

Hi, today I realized jQuery.map doesn't support hashes(associative arrays). I'm surprised, this is a behavior I'd assume it'd be included.

I added the changes to make this possible. In addition, added a third, optional, argument that is the return object. So it's possible to map on a existing object.

I made it so flexible, that one can map from an array to a hash, and the other way around as well.

This is mostly useful to collect the keys or values from a hash, or to map an array of elements to an object(like setArray).

It will handle array-like objects, and will treat them as arrays

I optimized the array concatenation, so now it's only done once, in the end, and now, there's no need for array-wrapping the returned data.

I think the length of the code remains more or less the same, so this should be doable.

I tested all the cases in IE6, FF2, Safari 3, and Opera 9.

I hope this helps

Attachments (3)
  • map-tests.diff (1.1 KB) - added by flesler April 22, 2008 11:38PM UTC.

    The tests for the test suite

  • map.diff (1.3 KB) - added by flesler March 29, 2008 05:27PM UTC.

    Proposal

  • map[5753].diff (1.1 KB) - added by flesler June 27, 2008 03:52PM UTC.

    Updated to match against [5753]

Change History (23)

Changed March 29, 2008 05:29PM UTC by flesler comment:1

Tried all these cases in IE6, FF2, Opera 9.25 and Safari 3 and they all worked well.

var keys = $.map( {a:1,b:2}, function( v, k ){
	return k;
}, [ ] );
alert(keys);

var values = $.map( {a:1,b:2}, function( v, k ){
	return v;
}, [ ] );
alert(values);

var mapped = $.map( document.getElementsByTagName('script'), function( v, k ){
	return v;
}, {length:0} );
alert(mapped);

var flat = $.map( Array(8), function( v, k ){
	return k % 2 ? k : [k,k,k];//try mixing array and regular returns
});
alert(flat);

Changed April 22, 2008 10:23PM UTC by joern comment:2

Could you convert that to the testsuite, too?

Changed April 28, 2008 02:00PM UTC by joern comment:3

need: ReviewCommit

Commited test in [5342].

Changed April 28, 2008 02:04PM UTC by joern comment:4

Your patch must be tested against the benchmark before applying it, as map is a core function of jQuery. Its raw speed has higher priority then being useful for other cases. If we can do both, even better.

Changed April 29, 2008 05:45PM UTC by flesler comment:5

Shouldn't you/we accept the change before adding the tests to the test suite ?

Or it will fail for others that haven't applied the proposed diff.

Changed April 29, 2008 05:46PM UTC by flesler comment:6

I meant you/we add the test, not accept.

I didn't mean to say it's on me to accept :)

Changed October 21, 2010 08:36PM UTC by SlexAxton comment:7

milestone: 1.2.41.5
owner: → flesler
priority: majorlow
status: newpending

Is this still something you guys think should make it into core? It's been a little while...

Changed November 11, 2010 11:09PM UTC by trac-o-bot comment:8

status: pendingclosed

Automatically closed due to 14 days of inactivity.

Changed February 19, 2011 02:11AM UTC by jordan@boedesign.com comment:9

I think the performance his is too much as per joern's comment:

http://jsfiddle.net/jboesch26/mmhwZ/4/

Changed February 19, 2011 07:27PM UTC by jordan@boedesign.com comment:10

I've made some changes to make sure this doesn't affect performance that much:

https://github.com/jquery/jquery/pull/238

Changed February 21, 2011 01:26AM UTC by rwaldron comment:11

#8328 is a duplicate of this ticket.

Changed February 21, 2011 01:27AM UTC by rwaldron comment:12

I've spent some time with the newest patch and perf comparing it to the existing, I'm indifferent about the perf. My issue is this...

// Provided test...

var keys = jQuery.map( {a:1,b:2}, function( v, k ){
	return k;
});

equals( keys.join(""), "ab", "Map the keys from a hash to an array" );


// But this is totally bonkers:
var keys = jQuery.map( {a:1,b:2, length: 2 }, function( v, k ){
	return k;
});

equals( keys.join(""), "ab", "Map the keys from a hash to an array, has a length property" );

// > "01" 
// expected: "ab"

Changed February 21, 2011 01:27AM UTC by rwaldron comment:13

milestone: 1.5
status: closedreopened

Changed February 21, 2011 01:28AM UTC by rwaldron comment:14

keywords: array mapneedsreview
milestone: → 1.next
version: 1.2.31.5

Reopening this for the current review

Changed February 27, 2011 06:54PM UTC by jboesch comment:15

Updated pull request can be found here: https://github.com/jquery/jquery/pull/252

Changed February 27, 2011 07:16PM UTC by rwaldron comment:16

status: reopenedpending

Did you look at the weird test results I posted previously?

Changed March 14, 2011 07:52AM UTC by trac-o-bot comment:17

resolution: → invalid
status: pendingclosed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

Changed March 21, 2011 03:50PM UTC by rwaldron comment:18

resolution: invalid
status: closedreopened

Changed March 21, 2011 03:51PM UTC by rwaldron comment:19

keywords: needsreview
milestone: 1.next1.6
priority: lowhigh

Changed March 30, 2011 06:03PM UTC by danheberden comment:20

owner: fleslerdanheberden
status: reopenedassigned

Changed April 10, 2011 08:46PM UTC by john comment:21

resolution: → fixed
status: assignedclosed

Landed.

Changed June 22, 2011 04:02PM UTC by yk comment:22

Why map to the object creates an array? I do not quite understand the history of this ticket, but the original patch was different behavior (map object to object) and I think it's more logical.

Changed June 22, 2011 04:11PM UTC by danheberden comment:23

For an end use standpoint, you're right - but internally, worrying about something else being returned from map besides an array is out of scope for the patch. Even then, the patch isn't designed to be full featured by any means - just a simple way to parse out keys or values into an array.

That's not to say someone can't make a plugin to offer the object to object map - not sure how that'd look recursively though..