bmurrell30
bmurrell30

Reputation: 715

for loop, Math.random() not acting as intended

I'm trying to code a dice game in javascript, using Math.random() to generate the values of the dice and an array to hold the current values of each individual die.

The problems I'm having are 1), the array ends up containing six numbers, and the loop only runs five times, and 2) I can't figure out how to get Math.random() to return different numbers on each dice roll.

Here's the function in question:

var array = []

function rollDice() {
    for (var i = 0; i <= 4; i++) {

        var roll = Math.floor(Math.random() * 6) + 1;

        array[i] = roll;
        //array.splice(i, 1, roll);
    }
}

You can see that 'i' iterates from zero to four in the loop (which equals five loops), and on each loop another random number is generated and inserted into the array at the equivalent place.

Regarding the number of items in the array: if I run the code as written above, I'll get something like this: [1, 2, 3, 4, 5, undefined]. If I run it using the commented line with the 'splice' function (which should remove the value stored at place 'i' and replace it with a new value,) I'll get [1, 2, 3, 4, 5, 6]. How is this happening if the loop is only running five times?

As to the difficulty I'm having getting the random numbers to change more reliably: I've tried the following in my loop...

var roll;
while (roll === hand[i] || roll === null) {
    roll = Math.floor(Math.random() * 6) + 1;
}

...in the hopes that 'roll' will keep spinning random numbers until it comes up with a different one, but no dice. Does anyone have any suggestions?

EDIT: I've added an explicit array declaration since I didn't make it clear enough that my array was in fact being explicitly declared before.

Upvotes: 0

Views: 4201

Answers (3)

Ismael Miguel
Ismael Miguel

Reputation: 4241

I got it working as you intended.

Mistakes in your code:

  1. array is undefined It's declared outside
  2. You are defining properties of an undefined variable.
  3. You are incorrectly using your while loop.
  4. Your function doesn't return anything. (Since the array is declared outside the loop, this is an unnecessary point.)

Here is revised code, using for:

function rollDice() {
    for (var i = 0, array = []; i <= 4; i++)
    {
        array[array.length] = Math.floor(Math.random() * 6) + 1;
    }
    return array;
}

Changes:

  1. Removed the useless roll variable.
  2. Used the property length to set the next position (similar to array.push(value) but faster).
  3. Declared the array
  4. Returned the value to be assigned outside the function

Using a do ... while loop:

function rollDice() {
    var array = [];
    do
    {
        array[array.length] = Math.floor(Math.random() * 6) + 1;
    }
    while ( array.length < 5 );
    return array;
}

Differences:

  1. The variable i isn't needed anymore.
  2. Direct declaration and increment of the array.length
  3. You only check the length at the end, which is one less useless check
  4. It's faster!!!

Regarding the (pevious) edit:

DON'T DO THAT!!!

Don't access a variable declared outside a function.

That's the most error-prone way of doing it!

Upvotes: 2

jfriend00
jfriend00

Reputation: 707456

Now that you have clarified that what you want is that successive calls to roll() will not repeat a value in that particular slot in the array, you can do that with this:

function rollDice() {
    var roll;
    for (var i = 0; i <= 4; i++) {
        // keep generate a new random value until it is different
        // than what was in this slot of the array before
        while ((roll = Math.floor(Math.random() * 6) + 1) === array[i]) {}
        array[i] = roll;
    }
}

Working demo:http://jsfiddle.net/jfriend00/frh3dyvd/

Upvotes: 0

garryp
garryp

Reputation: 5776

To return 5 unique numbers between 1 and 5 randomly:

var array = [1,2,3,4,5]

function rollDice() {
    array.sort(function(){
        return Math.random() - 0.5;
    });
}

Fiddle: http://jsfiddle.net/1wrgrr0v/1/

Upvotes: 0

Related Questions