Get random item from array, within a for loop, then remove from array

Question

I am writing a script that onload will add a class to a random 4 of 12 DIV's and then remove the ID of the DIV from the array.

I have an array setup with all 12 DIV ID's in it.

Sometimes when I reload the page, 4 DIV'S have that class and other times only 3 DIV's have that class.

Kinda stuck on why this is happening. I commented out the remove from array code to see if that was the issue, but still same problem.

Here is my code:

//Randomize Which Shoes Are Positive And Negative
function randomizeShoes(){
    allGroundShoes = new Array('ground_black_1','ground_black_2','ground_brown_1','ground_brown_2','ground_clown_1','ground_clown_2','ground_disco_1','ground_disco_2','ground_moccasins_1','ground_moccasins_2','ground_red_1','ground_red_2');
    for(var i=0; i < 4; i++){
        randomAllGroundShoes = allGroundShoes[Math.floor(Math.random() * allGroundShoes.length)];
        $('#'+randomAllGroundShoes+'').addClass('shoeNegitive');
        //randomShoeID = allGroundShoes.indexOf('randomAllGroundShoes');
        //if(randomShoeID != -1){ allGroundShoes.splice(randomShoeID, 1); }
    }
}

Math.floor(Math.random() * allGroundShoes.length) needs to be Math.floor(Math.random() * allGroundShoes.length - 1) because arrays start at 0. The length will return 12 but allGroundShoes[12] will not exist, as the last item would be allGroundShoes[11] since it starts at allGroundShoes[0]. — Felix Guo
– Felix Guo, Commented Nov 21, 2012 at 0:06
@MrXenotype It seems like that would be the case, but Math.random() returns a floating point number from 0.0 to less than 1 (i.e. 0 inclusive to 1 exclusive). See the MDN page for details. — GregL
– GregL, Commented Nov 21, 2012 at 0:11

gilly3 · Accepted Answer · 2012-11-21 00:36:45Z

2

When you remove the found element, you are passing in a string literal instead of the variable name:

allGroundShoes.indexOf('randomAllGroundShoes');

Since there is no element 'randomAllGroundShoes', the element would never be found, and no elements would ever be removed from the array.

It should be:

allGroundShoes.indexOf(randomAllGroundShoes);

But, you are doing the same thing more than once. You don't need to check allGroundShoes.indexOf() at all. You could just store the random number in a variable and reference it again. But, even that is more than you need. Just call splice() to get your value:

randomAllGroundShoes = allGroundShoes.splice(
    Math.floor(Math.random() * allGroundShoes.length), 1)[0];
$('#'+randomAllGroundShoes).addClass('shoeNegitive');

This way, you retrieve your value and remove it from the array in one step - no additional lookup required.

jsfiddle.net/kRNTg

answered Nov 21, 2012 at 0:36

gilly3

92.6k26 gold badges148 silver badges179 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Bergi Over a year ago

Just what I said :-) Yet I'd recommend to use a variable for the index instead of this lengthy one-liner (why use randomAllGroundShoes at all, we could move the random selection into the jQuery selection) ...

GregL · Accepted Answer · 2012-11-21 00:07:41Z

0

Your remove code is the problem, I believe. Try changing the last two commented out lines to be:

randomShoeID = allGroundShoes.indexOf(randomAllGroundShoes);
if(randomShoeID != -1){ allGroundShoes = allGroundShoes.splice(randomShoeID, 1); }

answered Nov 21, 2012 at 0:07

GregL

38.2k8 gold badges65 silver badges68 bronze badges

2 Comments

gilly3 Over a year ago

After this operation, allGroundShoes will contain 1 element - the element that was just found.

GregL Over a year ago

@gilly3 Well noted, sir. I had forgotten splice returns the removed array element(s).

Atif · Accepted Answer · 2012-11-21 00:16:02Z

0

Possibly, that same number might be getting generated twice.

In that case you have to have a checking mechanism if that number is already used or not.

What you can do is generate an array of 4 unique random number and iterate through that to set the classes.

var unqiue_arr = []
while(arr.length < 8){
  var randomnumber=Math.floor(Math.random()*12)
  var found=false;
  for(var i=0;i<3;i++){
    if(unqiue_arr[i]==randomnumber){found=true;break}
  }
  if(!found)unqiue_arr[unqiue_arr.length]=randomnumber;
}

edited Nov 21, 2012 at 0:16

answered Nov 21, 2012 at 0:05

Atif

10.9k22 gold badges68 silver badges97 bronze badges

3 Comments

GregL Over a year ago

You would think that, hey? But Math.random() never gives a result of 1.0. See this MDN link for details.

Atif Over a year ago

@GregL +1 actually yes, in that case is should be the second reason., editing my answer

gilly3 Over a year ago

He is already removing the element from the array (or at least, he's trying to). That is enough to prevent duplicates - as long as it works. This nested loop approach is quite inefficient.

Bergi · Accepted Answer · 2012-11-21 00:35:36Z

Your code with the commented out lines has the problem that when the items are not removed, they might get selected twice or more by the random selection.

Your commented out lines had the same problem. You did search for allGroundShoes.indexOf('randomAllGroundShoes'); which (being a string instead of the variable) was never contained in the array.

Use this code instead:

function randomizeShoes(){
    allGroundShoes = ['ground_black_1','ground_black_2','ground_brown_1','ground_brown_2','ground_clown_1','ground_clown_2','ground_disco_1','ground_disco_2','ground_moccasins_1','ground_moccasins_2','ground_red_1','ground_red_2'];
    for (var i=0; i<4; i++){
        var randomShoeID = Math.floor(Math.random() * allGroundShoes.length);
        var randomAllGroundShoes = allGroundShoes[randomShoeID];
        $('#'+randomAllGroundShoes+'').addClass('shoeNegitive');
        allGroundShoes.splice(randomShoeID, 1);
    }
}

There is no need for searching the index of an item which you already know, and there is no need for checking against -1 as you know the item is contained in the array from which you just pulled it out.

Collectives™ on Stack Overflow

Get random item from array, within a for loop, then remove from array

4 Answers 4

1 Comment

2 Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

2 Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related