0

So what I am trying to do is create a random array of 5 elements, those elements should be filled with numbers from 1 to 6 and they shall not repeat, I can't tell where my logic is wrong.

void genNumber(int vet[]){
   int max, i, j, atual;
   srand(time(NULL));

   max = 7;
   for (i=0;i<5;i++){
      vet[i] = rand() % max;
      while(vet[i] == 0){
         vet[i] = rand() % max;
      }

      for(j=0;j<i;j++){
         atual = vet[j];
         while((vet[i] == atual)||(vet[i] == 0)){
            vet[i] = rand() % max;
            atual = vet[j];
         }
      }
   }
}

Update: Fixed

void genNumber(int vet[]){
int max, i, j;
srand(time(NULL));

max = 7;
for (i=0;i<5;i++){
    vet[i] = rand() % (max-1) + 1;

    for(j=0;j<i;j++){
        while(vet[j] == vet[i]){
            vet[i] = rand() % (max-1) + 1;
            j = 0;
        }
    }
}
}
Skalwalker
  • 299
  • 3
  • 23
  • What's the error message? – cadaniluk Oct 22 '15 at 15:24
  • @cad: If they are unsure where their *logic* is wrong, it's likely it compiles without errors, but doesn't work as expected. – Joey Oct 22 '15 at 15:25
  • The second `atual = vet[j];` seems redundant/erroneous. – JimmyB Oct 22 '15 at 15:26
  • Instead of `while(vet[i] == 0){ ...` you can just use `vet[i] = rand() % (max-1) + 1;`. – JimmyB Oct 22 '15 at 15:29
  • Note that you can also initialize your array with, e.g., `{ 1, 2, 3, ... }` and then just randomly swap positions. – JimmyB Oct 22 '15 at 15:33
  • That would work but they have 6 possibilites of numbers and the array can only have 5. – Skalwalker Oct 22 '15 at 15:34
  • Then just take the first 5 array elements after randomizing the order of the 6-element array :) – JimmyB Oct 22 '15 at 15:36
  • Sorry, I think I am not expressing myself precisely. It is a 5 array element that contain random numbers from 1 to 6 without repeating any – Skalwalker Oct 22 '15 at 15:39
  • I think I got you right. And the quickest solution is still the permutation thing I mentioned above. – JimmyB Oct 22 '15 at 15:44
  • @HannoBinder: Note that to actually shuffle those and get an unbiased sampling you need to take care of what positions to swap with what others. Simply randomly swapping items with random positions will create a bias. – Joey Oct 22 '15 at 15:44
  • @Joey is right. And basically the answer to your question is already [here](http://stackoverflow.com/questions/7902391/generate-an-uniform-random-permutation). – JimmyB Oct 22 '15 at 15:51
  • Thanks guys, all very good answers, but I still need to solve it using loops and basics. I updated my question with the right code – Skalwalker Oct 22 '15 at 16:41
  • Do _not_ "updated my question with the right code". Instead, post that as an answer - you can even accept your own answer. As it stands, that update is a weak solution. – chux - Reinstate Monica Oct 22 '15 at 16:49

3 Answers3

1

The logical flaw is in the way you produce a new random number when a duplicate is found.

Imagine you already have vel = {1,2,0,0,0,...} and are trying to find a number for vel[2]. If you randomly draw a 2, you'll find it's already there and draw again. But if you draw a 1 this time you won't notice, because you only compare to the last value seen, 2 in the example. So you'd get vel = {1,2,1,...}.

"Solution": Every time you draw a new random number you have to compare it against all numbers already in the list.

Another way of solving this is the one I tried to outline in the comments: You have to keep the information about which numbers are still valid for a draw somewhere. You can either use the "output" array for that as you're doing now, or you can use another store from which you "remove" an entry once it was drawn.

JimmyB
  • 12,101
  • 2
  • 28
  • 44
  • Yes, that's the correct way to implement his choice of algorithm, but that's a terrible algorithm to use: it will waste huge amounts of time rejecting bad numbers. The correct thing to do (as Linus points out below) is to fill an array with 1..6, shuffle it, then use the first five elements. – Lee Daniel Crocker Oct 22 '15 at 16:45
0

when generating a random number,you have to check if it is found in the array,so iterate over the array and check.if the number is found,generate another random number and check,until the new number is not found in the array.after that assign it.and repeat.

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int check(int vet[],size_t size,int val);

void genNumber(int vet[],size_t size);

int main(void)
{
    srand(time(NULL));

    int vet[5];

    size_t size = 5;

    genNumber(vet,size);

    for( size_t n = 0 ; n < size ; n++ )
    {
        printf("%d ",vet[n]);
    }
}

void genNumber(int vet[],size_t size)
{
    int num = rand() % 6 + 1;

    vet[0] = num;

    for( size_t n = 1 ; n < size ; n++ )
    {
        num = rand() % 6 + 1;
        while( 1 )
        {
            if( check(vet,n,num) )
            {
                num = rand() % 6 + 1;
            }
            else
            {
                vet[n] = num;
                break;
            }
        }
    }
}

int check(int vet[],size_t size,int val)
{
    for( size_t n = 0 ; n < size ; n++ )
    {
        if( vet[n] == val )
        {
            return 1;//FOUND
        }
    }
    return 0;//NOT FOUND
}
machine_1
  • 4,266
  • 2
  • 21
  • 42
0

As an alternative you can fill an array with a value from 1 to 6. Afterwards you can shuffle the array at random indicies.

#define MAX 6

void swap (int *a, int *b) {
  int temp = *a;
  *a = *b;
  *b = temp;
}

void genNumber(int *vet) {
  int i;

  for(i=0;i<MAX;i++) {
    vet[i] = i+1;
  }

  for(i = MAX-1;i > 0;i--) {
    // Pick a random index from 0 to i
    int j = rand() % (i+1);
    // Swap vet[i] with the element at random index
    swap(&vet[i], &vet[j]);
  }
}

Also you shouldn't call srand(time(NULL)); inside of the genNumber function if you intend to call it more than once, becaues otherwise it will give you the same numbers if you call it more than once a second.

Afterwards just use the first five elements from the array. So you would call it like this from main

int vet[MAX] = {0};
srand(time(NULL));
genNumber(vet);
for(i=0;i<5;i++) {
  printf("after num %d\n", vet[i]);
}
Linus
  • 1,516
  • 17
  • 35