CODEWITHSUNDEEP
javascript
ryandlf
ryandlf

Reputation: 28545

Using array.splice inside Array prototype

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?

Upvotes: 0

Views: 375

Answers (3)

ryandlf
ryandlf

Reputation: 28545

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.

Upvotes: 1

RobG
RobG

Reputation: 147363

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.

Upvotes: 0

kennebec
kennebec

Reputation: 104760

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
*/

Upvotes: 0

Related Questions