Extend makeArray

[flesler]

Hi, don't you think makeArray should be a bit smarter?

Instead of doing $.makeArray( foo [] ) all the time, it should handle that internally.

$.fn.setArray does a lot of checks that IMO, belong to $.makeArray. By improving it, you can remove those pesky checks from many places.

This version handles pretty much any kind of argument, without using too many checks.

I know that internal functions have to as fast as possible, I optimized this as much as I could, but well, in the end, it's up to you (core team).

[flesler]

The check for array.call can be removed, if there's no interest on supporting functions, I just added because it may happen. !array.split can be changed for typeof array != 'string' if object detection is not the best option.

[flesler]

I made a swift test runner, and after a few fixes (specially for safari), they all passed on IE6, FF2, Opera 9 and Safari 3.

These are the tests:

//nodelist
var scripts = document.getElementsByTagName('script');
if( $.makeArray(scripts).slice(0,1)[0].nodeName != 'SCRIPT' )
	alert('error-1');
//arguments
if( (function(){ return $.makeArray(arguments); })(1,2).join('') != '12' )
	alert('error-2');
//real array
if( $.makeArray([1,2,3]).join('') != '123' )
	alert('error-3');
//empty
if( $.makeArray().length != 0 )
	alert('error-4');
//number
if( $.makeArray( 0 )[0] != 0 )
	alert('error-5');
//string
if( $.makeArray( 'foo' )[0] != 'foo' )
	alert('error-6');
//boolean
if( typeof $.makeArray( true )[0] != 'boolean' )
	alert('error-7');
//node
if( $.makeArray( scripts[0] )[0].nodeName != 'SCRIPT' )
	alert('error-8');
//array-like object
if( $.makeArray( {length:2, 0:'a', 1:'b'} ).join('') != 'ab' )
	alert('error-9');
//childnodes array
if( $.makeArray( document.documentElement.childNodes ).slice(-1)[0].nodeName != 'HEAD' )
	alert('error-10');
//function (tricky, they have length)
if( $.makeArray( function(){ return 1;} )[0]() != '1' )
	alert('error-11');

[flesler]

You might say: "ok, slice is not used for regular arrays". I benchmarked my option against slice and concat (with an empty page w/o jquery or anything else) and the native alternative was only faster on Safari 3. Firefox 2 was actually like 10x times faster with a reversed for.

This is the benchmark:

(function(){
	var
		arr = Array(30),
		times = 1e5,
		i, t;
	t = times;
	console.time('mine');
	while( t-- ){
		var i = arr.length
		while( i )
			arr[--i] = arr[i];
	}
	console.timeEnd('mine');
	t = times;
	console.time('native');
	while( t-- )
		arr.slice();
	console.timeEnd('native');
})();

I made a fake console with time() and timeEnd() for all but Firefox. Results:

Firefox 2.0.0.13 (turned firebug off)
    * mine: 641ms
    * native: 5312ms
IE 6
    * mine: 2078ms
    * native: 2422ms
Opera 9.22
    * mine: 1656ms
    * native: 1844ms
Safari 3.0.4
    * mine: 1281ms
    * native: 344ms

This should be considered for the rest of the code as well, native methods might not always be the fastest.

[flesler]

One correction: the while loop was an experiment, i was comparing loops. The real loop (the one in the code) is:

	t = times;
	console.time('mine');
	while( t-- ){
		for( i = arr.length; i; )
			arr[--i] = arr[i];
	}
	console.timeEnd('mine');

Benchmarked all back and the numbers were the same.

[joern]

Could you rewrite those tests using the jQuery testsuite?

[flesler]

Commited the required tests on [5284].

[joern]

Commited patch: [5314]. Keeping ticket open until a few other places make use of the new implementation.

[flesler]

Simplified the code using the new makeArray where possible, at [5137]

[joern]

That should have been [5317].

[joern]

I wonder if makeArray can really replace all those checks in the init method:

// Watch for when an array-like object, contains DOM nodes, is passed in as the selector
(selector.jquery || selector.length && selector != window && !selector.nodeType
 && selector[0] != undefined && selector[0].nodeType) && jQuery.makeArray( selector )

Odds are good that the testsuite doesn't really cover all of them.

The other changes look fine.

[flesler]

I just checked, and 'setArray' is only used inside 'init'. 'init' is only used inside jQuery. In consequence, setArray needs to handle whatever $() handles. String selectors/html and functions are sniffed out before, that only leaves us with: - Arrays - Array-like (jQuery, nodelist) - DOM elements

The first two are of course handled. DOM elements are checked first, prior to this. We could actually remove that prior check because makeArray handles it well. Of course, the this[0] = selector part, is faster than a whole makeArray+setArray, so that if can be kept to speed up that case.

This enhancement to makeArray, actually opens up some new possibilities. Like $( 2 ), $( new Date ), $( {foo:'bar'} ), $( /[abc]/ ), $( new MyClass? ), etc.

I think this is really good!

The only problem that might arise is with window, as it has length. We can add a " && !array.location " or .setInterval, to detect the window. The !array.call can be removed if we don't need to support functions.

[flesler]

Fixed the detection for window, and added a test case for window and a regex at [5318].