Using array.splice inside Array prototype

Question

Array.prototype.move = function(oldIndex, newIndex) {
 var val = this.splice(oldIndex, 1);
 this.splice(newIndex, 0, val[0]);
}
//Testing - Change array position
var testarray = [1, 2, 3, 4];
testarray.move(3, 0);
console.log(testarray);

This produces an error "this.splice is not a function" yet it returns the desired results. Why?

• ryandlf: Firefox. Hmm. its definitely giving me an error.
• user1106925: Works for me in Firefox jsfiddle.net/XgrJF/1 with no errors. Are you testing in Firebug?
• ryandlf: Yes testing in firebug. I wonder why its giving me the error.
• user1106925: @ryandlf: Sometimes Firebug has bugs. Make sure you've updated to the latest version. Does the jsFiddle in my comment above give you errors?
• ryandlf: No, it doesn't. I'll see about updating firebug.

Answer 1

Array.prototype.move = function(oldIndex, newIndex) {
 if(Object.prototype.toString.call(this) === '[object Array]') {
 if(oldIndex && typeof oldIndex == 'number' && newIndex && typeof newIndex == 'number') {
 if(newIndex > this.length) newIndex = this.length;
 this.splice(newIndex, 0, this.splice(oldIndex, 1)[0]);
 }
 }
};

For some reason, the function is being called by the called by the document on load (still haven't quite figured that one out). I added a few checks to verify that this = an array, and then also reset the new index to be equal to the total size if the supplied int was greater than the total length. This solved the error issue I was having, and to me is the simplest way to move objects around in an array. As for why the function is being called onload must be something to do with my code.

• RobG: That's still pretty ordinary. If either index is the number zero, the second if test will fail. Array index names are just ordinary object properties, they are strings, so testing that the supplied arguments are numbers doesn't make sense. And string zero will pass the && test. You have't solved the issue of missing members or different results where the arguments are swapped. And now it's a lot more code that the non-split alternative. Why make things harder than they need to be?

Answer 2

You don't need the placeholder variable-

Array.prototype.move = function(oldIndex, newIndex) {
 this.splice(newIndex, 0, this.splice(oldIndex, 1)[0]);
}
var a=[1,2,3,4,9,5,6,7,8];
a.move(4,8);
a[8]
/* returned value: (Number)
9
*/

• RobG: The above is order dependent: a.move(8,4) != a.move(4,8). For it to be consistent, the removed value must be temporarily stored.
• user1106925: Doesn't really address the issue in the question, does it?

Answer 3

Adding properties to built–in objects is not a good idea if your code must work in arbitrary environments. If you do extend such objects, you shouldn't use property names that are likely to be used by someone else doing the same or similar thing.

There seems to be more than one way to "move" a member, what you seem to be doing can be better named as "swap", so:

if (!Array.prototype.swap) {
 Array.prototype.swap = function(a, b) {
 var t = this[a];
 this[a] = this[b];
 this[b] = t; 
 }
}

I expect that simple re-assignment of values is more efficient than calling methods that need to create new arrays and modify the old one a number of times. But that might be moot anyway. The above is certainly simpler to read and is fewer characters to type.

Note also that the above is stable, array.swap(4,8) gives the same result as array.swap(8,4).

If you want to make a robust function, you first need to work out what to do in cases where either index is greater than array.length, or if one doesn't exist, and so on. e.g.

var a = [,,2]; // a has length 3
a.swap(0,2);

In the above, there are no members at 0 or 1, only at 2. So should the result be:

a = [2]; // a has length 1

or should it be (which will be the result of the above):

a = [2,,undefined]; // a has length 3

or

a = [2,,,]; // a has length 3 (IE may think it's 4, but that's wrong)

Edit

Note that in the OP, the result of:

var b = [,,2];
b.move(0,2);

is

alert(b); // [,2,];

which may not be what is expected, and

b.move(2,0);
alert(b); // [2,,];

so it is not stable either.