CODEWITHSUNDEEP
javascriptarrays
Nasco.Chachev
Nasco.Chachev

Reputation: 676

Losing the logic in my Javascript program

I have an issue with Javascript. I want to make the program , which generates 6 random numbers ( from 1 to 49 ).Anyway im doing it with for loop and it works. But when I try to find if any of those 6 numbers has a duplicate number , to replace it with a new random number. I've looked at js array duplicate functions , Im applying it , but it seems my fault is in the logic , still beginner with js , so please help me :)

js code: 

var arr = new Array(7);
var numbers = [];

for(var i=1; i<arr.length;i++){
    numbers[i] = Math.floor(Math.random()*48)+1;
    if(numbers[i] == numbers[i+1]){
        arr.indexOf(numbers[i]);
        arr.push(Math.floor(Math.random()*48));
    }
}

i`m trying to apply indexOf ( to find the duplicate number's index, and after that to replace it with new element via push()). Please give me some advises how to improve on my programming logic skills :(

Upvotes: 0

Views: 58

Answers (4)

Will
Will

Reputation: 1768

This statement will never be true:

if(numbers[i] == numbers[i+1]){

When you start, numbers array is empty. Then you assign one random to element 0. Then you try to compare element 0 in numbers to 1, but numbers[1] will be undefined.

The best way to do this is to start with an empty arr array, meaning length of 0. Loop until length equals 6. In the loop, generate a random number and only push the value into the arr array if the number doesn't exist in there.

UPDATE:

After seeing some of the answers, I think this is probably the simplest:

var arr = [],
    num;

while (arr.length < 6) {
    num = Math.floor(Math.random() * 48) + 1;
    if (arr.indexOf(num) === -1) {
        arr.push(num);
    }
}

console.log(arr);

Upvotes: 1

Robus
Robus

Reputation: 465

for (var a=[],i=1;i<40;++i) a[i]=i;
     function shuffle(array) {
       var tmp, current, top = array.length;
       if(top) while(--top) {
       current = Math.floor(Math.random() * (top + 1));
       tmp = array[current];
       array[current] = array[top];
       array[top] = tmp;
      }
   return array;
  }

a = shuffle(a);

Random Numbers , No repeats

Upvotes: 1

Tobias
Tobias

Reputation: 7771

First, JavaScript arrays are zero-indexed, so the first index is 0, not 1. Although it is valid to skip the first index and use the second to seventh indices instead, you will usually want to avoid that.

Second, why do you check numbers[i] against numbers[i + 1]? At that point, numbers[i + 1] won't have been set yet as it will be set in the next iteration. Additionally, even if this problem did not exist, your code would fail to catch this sequence: [ 5, 3, 5 ].

Third, I am not quite sure why you use two different arrays. Although you definitely can, there is no good reason to do so.

I will suggest another way to solve your problem and add comments to the steps necessary. I hope these will help you to understand this solution to your problem.

// Create an array for the chosen numbers
var numbers = [];

// Loop to generate 6 elements, zero-based
for(var i = 0; i < 6; i++) {
   // This variable will hold the number we choose
   var number;

   // This is a do-while loop which generates a random integer
   do {
      // Generate a random integer between 1 and 49 (incl.)
      number = Math.floor(Math.random() * 48) + 1;
   } while(numbers.indexOf(number) != -1); // Exit loop once the number is not contained in the array

   // Store the number in the array
   numbers[i] = number;
}

This algorithm is short (8 SLOC) and pretty efficient.

Upvotes: 1

Allicnam
Allicnam

Reputation: 88

This is one way you could do it:

var numbers = [];

for(i=0;i<6;i++){
 numbers[i] = Math.floor(Math.random()*48)+1;
//this loop will help us check all of the already assigned numbers.
  for(x=0;x<i;x++){
    if(numbers[x] == numbers[i]){
      i--;
    }
  }
}

This might not be the most efficient and optimal way to do it, but it sure works and it will give you no repetition on the numbers now that it will loop untill it finds a different number. Its a fast and short solution, yet not the most efficient. Hope this helps =).

Upvotes: 1

Related Questions