1

I have this strange problem when I shuffle an array in javascript and I have no clue what's the problem. Can somebody help me?

When I shuffle an array like this

[1,2,3,4,5,6,7,8,9,10]

I get a null value, like this

[null,10,1,8,9,3,2,7,6,4]

This is the code (http://jsfiddle.net/2m5q3d2j/):

Array.prototype.suffle = function () {
    for (var i in this) {
        var j = Math.floor(Math.random() * this.length);
        this[i] = this[j] + (this[j] = this[i], 0);
    }

    return this;
};
LiNoimi Semain
  • 13
  • 2
  • Well, the first thing is, don't use `for-in` to loop through arrays (without safeguards, and there are better ways). More: http://stackoverflow.com/questions/9329446/for-each-over-an-array-in-javascript/9329476#9329476 – T.J. Crowder Nov 29 '14 at 12:47
  • 2
    It's not the problem, but playing games with the evaluation order like that is just completely unnecessary. Use the flippin' temp variable. – T.J. Crowder Nov 29 '14 at 12:54
  • @T.J.Crowder: The link you're looking for is https://stackoverflow.com/questions/500504/why-is-using-for-in-with-array-iteration-such-a-bad-idea :-) – Bergi Nov 29 '14 at 13:20
  • @Bergi: I think my answer above addresses those points, and much more. :-) – T.J. Crowder Nov 29 '14 at 13:47

1 Answers1

4

Because you're adding an enumerable property (shuffle) to Array.prototype, if you insist on iterating with for-in, you need to add a hasOwnProperty test:

Array.prototype.shuffle = function () {
    for (var i in this) {
        if ( this.hasOwnProperty(i) ) {
           var j = Math.floor(Math.random() * this.length);
           this[i] = this[j] + (this[j] = this[i], 0);
        }
    }

    return this;
};

Otherwise I would rather suggest:

Array.prototype.shuffle = function () {
    for (var i=0; i < this.length; i++) {
        var j = Math.floor(Math.random() * this.length);
        this[i] = this[j] + (this[j] = this[i], 0);
    }

    return this;
}

http://jsfiddle.net/2m5q3d2j/5/

You can also use Object.defineProperty to create the property, to avoid making it enumerable, on ES5+ engines.

M. Page
  • 2,694
  • 2
  • 20
  • 35