Shuffling Array Javascript

Solution 1:

Your new_deck just contains indexes, not values and you aren't removing the previously chosen values before you choose the next random value. .pop() does not take an argument so you can't remove a specific index with it - you would have to use .splice() for that. All in all, you aren't getting a new array of your values for several reasons.

If you just want an easy way to randomize an array without reinventing the wheel and knowing that the randomness actually works well you can use this:

functionfisherYates ( myArray ) {
  var i = myArray.length;
  if ( i == 0 ) returnfalse;
  while ( --i ) {
     var j = Math.floor( Math.random() * ( i + 1 ) );
     var tempi = myArray[i];
     var tempj = myArray[j];
     myArray[i] = tempj;
     myArray[j] = tempi;
   }
}

You can read about the Fisher-Yates method here:http://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle.

You can read about why some other randomization methods don't work as well you can read here: http://www.robweir.com/blog/2010/02/microsoft-random-browser-ballot.html.

So, you would replace this:

for (r=0;r<deck.length;r++){
            var randomindex = Math.floor(Math.random()*deck.length);
            new_deck.push(randomindex)
            deck.pop(randomindex)
            }

        document.write("deck = ")

        for (r =0; r<deck.length; r++){
            document.write(deck[r]);
            }

        document.write("</br>")

        document.write("new deck = ")

        for (r=0; r<new_deck.length; r++){
            document.write(new_deck[r]);
            }

with this:

        document.write("deck = ")

        for (r =0; r<deck.length; r++){
            document.write(deck[r]);
            }

        document.write("</br>")

        document.write("shuffled deck = ")

        // shuffle the deck and then output itfisherYates(deck);

        for (r=0; r<deck.length; r++){
            document.write(deck[r]);
            }

Solution 2:

Another super easy array sort technique:

 deck.sort(function() { return Math.random()-0.5})

Solution 3:

You'll want to change your shuffle for this:

while(deck.length > 0 ){ //for loop won't workvar randomindex = Math.floor(Math.random()*deck.length);
    new_deck.push(deck[randomindex]);//Move onto new stack
    deck.splice(randomindex,1); //Take from old.
}

Here's why the for loop won't work: Say deck.length = 4

Start of the for loop: r = 0

One item is pushed onto new stack & popped: deck.length = 3

r is incremented r = 1

r is less than deck.length, continue

One item is pushed onto new stack & popped: deck.length = 2

r is incremented r = 2

r is no longer less than deck.length, the loop has finished but only half the elements where transferred!

Solution 4:

you are getting a random value, not a random index into the deck

for (r=0;r<deck.length;r++){
    var randomindex = Math.floor(Math.random()*deck.length);
    new_deck.push(deck[randomindex]);
}

Solution 5:

The first problem begins in the for loop where you randomize the deck. You need to create a new variable before the loop containing the deck's length. See:

for (r=0;r<deck.length;r++){
    var randomindex = Math.floor(Math.random()*deck.length);
    new_deck.push(randomindex)
    deck.pop(randomindex)
}

What you have going on here is that r increases by 1 at the same time as deck.length is decreasing by 1 (due to the pop in the for loop). So, that's why you're only getting four "cards" in your shuffled deck. The second problem is that you're not pushing a card into the new deck, you're pushing the randomindex itself.

This code should do the trick:

var i = deck.length;
for (r=0;r<i;r++){
    var randomindex = Math.floor(Math.random()*deck.length);
    new_deck.push(deck.splice(randomindex, 1));
}

(P.S.: Originally, neither of your statements in the above for loop had semicolons at the end.)

Edit: You could also use a while() loop. See meiamsome's answer.

Edit 2: As Joe mentioned below, pop() doesn't take an argument. splice() does what you're trying to accomplish.

Share