How to select a set of unique random numbers (without duplicates) using the jQuery.inArray method?

Question

This is a Javascript / jQuery question:

I am trying to create six unique random numbers from 1 to 21 (no duplicates) using the jQuery.inArray method. These six numbers will then be used to select six .jpg files from the group named logo1.jpg through the logo 21.jpg.

Can someone tell me what I'm doing wrong here?

<div id="client-logos"></div> <script src="http://code.jquery.com/jquery-1.5.js"></script> <script type="text/javascript"> Show = 6; // Number of logos to show TotalLogos = 21; // Total number of logos to choose from FirstPart = '<img src="/wp-content/client-logos/logo'; LastPart = '.jpg" height="60" width="120" />'; r = new Array(Show); // Random number array var t=0; for (t=0;t<Show;t++) { while ( jQuery.inArray(x,r) { var x = Math.ceil(Math.random() * TotalLogos); }); r[t] = x; var content = document.getElementById('client-logos').innerHTML; document.getElementById('client-logos').innerHTML = content + FirstPart + r[t] + LastPart; } </script> 

Thanks in advance...

Answer 1

you have a few questions:

• global window variables

• an array declared with the keyword new , not literal

• trying to use variables before declaring them

• jQuery.inArray is used incorrectly ( inArray returns a number , not true or false )

• inefficient code with a while , which theoretically could lead to an infinite loop

now the second in combination with the third is the main problem here, since the first time you check x in an array, it is undefined (you define it only inside if with var , therefore x is first undefined) and thus it matches the first an array element (which is undefined , as you declared r with new Array(6) ), and the inArray function returns 1, which leads to an infinite loop.

There are a few things you could do to fix this code, but I think that a complete rewrite with a different approach might be better and doesn't require jQuery at all.

This modified version of your code should work fine:

 var Show = 6, // Number of logos to show TotalLogos = 21, // Total number of logos to choose from FirstPart = '<img src="/wp-content/client-logos/logo', LastPart = '.jpg" height="60" width="120" />', array = [], // array with all avaiilable numbers rnd, value, i, logosElement = document.getElementById('client-logos'); for (i = 0; i < TotalLogos; i++) { // arrays are zero based, for 21 elements you want to go from index 0 to 20. array[i] = i + 1; } for (i = 0; i < Show; i++) { // pick numbers rnd = Math.floor(Math.random() * array.length); value = array.splice(rnd,1)[0]; // remove the selected number from the array and get it in another variable logosElement.innerHTML += (FirstPart + value + LastPart); } 

To explain a little what I did here:

• initialize the array with all possible values ​​(numbers 1 to 21)

• run the loop only as many times as you want to choose.

• generates a random number from 0 to the maximum index available in your array of numbers

• remove the number from this index from the array using splicing, and then use it to create a string to call innerHTML ( splicing returns elements removed from the array as another new array ).

• In addition, the logosElement variable logosElement cached at the beginning, so you only perform one DOM lookup for an element.

There are many ways to rewrite the code and even clean it up, but I decided that it would be the cleanest way to help you solve the problem (and it’s safe for a cross browser! No need to add jQuery unless you need other functionality)

Answer 2

 // Initial number var x = Math.ceil(Math.random() * TotalLogos); // Keep searching until a unique number is found while ($.inArray(x, r) > -1) { x = Math.ceil(Math.random() * TotalLogos); } // If it unique, set it r[t] = x; 

Answer 3

The immediately obvious reason why it does not work, besides the extra closing brackets after your while , is a little misunderstanding as to how the $.inArray method $.inArray . $.inArray returns the first index of the matching value in the array, or -1 if it is not found. -1 is a "true" value, that is, your while will continue if a random number is not in the array. In fact, it will continue to execute until it finds a number at the 0th position of the array.

To fix this specific problem, you need to check that it is greater than -1, and also set x var to a random number before the loop:

 var x = Math.ceil(Math.random() * TotalLogos); while ($.inArray(x, r) > -1) { x = Math.ceil(Math.random() * TotalLogos); } r[t] = x; 

In the comments on your question, Pointy also mentions Shisherle Fisher-Yates. This shuffling method may give you a better spread than the approach you are using.