Initialising 2-D array with random values

[devnar]

I have tried to come up with a 2-D (4 rows and 4 columns) array which holds values from 0 to 15, with none of the values used more than once.

I'm posting only the code snippet.
please lemme know if the logic is correct.

int HIGH=16; /*(Max value of an array element)+1*/
 int done[15]; /* Shows which of the 16 numbers have already been used*/
 int i, j, val;
 time_t secs;
 time(&secs); /*assign 'secs' with the current time*/
 srand((unsigned)secs); /*initialize the seed for rand()*/
 for(i=0;i<15;i++)
  done[i]=0; /* None of the 16 numbers have been used yet*/

  for(i=0;i<4;)
  {
     for(j=0; j<4;)
     {
        val=rand()%HIGH;        /*generates a number between 0 and 15*/
        if(!done[val])  /*if number hasn't been used already*/
       {
         a[i][j]=val;
         done[val]=1; /* mark the particular number as done*/
         j++; /* increment j only if the current array position gets assigned a number*/
       }
     }
     i++;
  }

[VernonDozier]

I have tried to come up with a 2-D (4 rows and 4 columns) array which holds values from 0 to 15, with none of the values used more than once.

I'm posting only the code snippet.
please lemme know if the logic is correct.

int HIGH=16; /*(Max value of an array element)+1*/
 int done[15]; /* Shows which of the 16 numbers have already been used*/
 int i, j, val;
 time_t secs;
 time(&secs); /*assign 'secs' with the current time*/
 srand((unsigned)secs); /*initialize the seed for rand()*/
 for(i=0;i<15;i++)
  done[i]=0; /* None of the 16 numbers have been used yet*/

  for(i=0;i<4;)
  {
     for(j=0; j<4;)
     {
        val=rand()%HIGH;        /*generates a number between 0 and 15*/
        if(!done[val])  /*if number hasn't been used already*/
       {
         a[i][j]=val;
         done[val]=1; /* mark the particular number as done*/
         j++; /* increment j only if the current array position gets assigned a number*/
       }
     }
     i++;
  }

Your done array has 15 elements and thus can handle indexes from 0 to 14. val has a potential range of 0 - 15. You have this on line 15:

if(!done[val])

which will be this if val is 15:

if(!done[15])

which, since the legal indexes are from 0 to 14, is out of range so you could have a segmentation fault. Aside from the segmentation fault issue, done[15] is never initialized to 0 in these lines:

for(i=0;i<15;i++)
  done[i]=0; /* None of the 16 numbers have been used yet*/

[devnar]

Thanks for the heads up VernonDozier. Actually, i was tinkering with the code before posting and i just forgot to change that. i'll edit that statement.

what about the logic from line 10 to line 23? is it correct? will it initialize the array with random values?

[devnar]

Edited code:
Changed int done[15] to int done [16]

int HIGH=16; /*(Max value of an array element)+1*/
 int done[16]; /* Shows which of the 16 numbers have already been used*/
 int i, j, val;
 time_t secs;
 time(&secs); /*assign 'secs' with the current time*/
 srand((unsigned)secs); /*initialize the seed for rand()*/
 for(i=0;i<15;i++)
  done[i]=0; /* None of the 16 numbers have been used yet*/

  for(i=0;i<4;)
  {
     for(j=0; j<4;)
     {
        val=rand()%HIGH;        /*generates a number between 0 and 15*/
        if(!done[val])  /*if number hasn't been used already*/
       {
         a[i][j]=val;
         done[val]=1; /* mark the particular number as done*/
         j++; /* increment j only if the current array position gets assigned a number*/
       }
     }
     i++;
  }

[Aia]

Some to consider,

int HIGH=16; /*(Max value of an array element)+1*/

That could be instead a constant

#define HIGH 16

Now you can use HIGH as the subscript size of done,

int done[HIGH];

All that,

time_t secs;
time(&secs); /*assign 'secs' with the current time*/

srand((unsigned)secs); /*initialize the seed for rand()*/

can be substituted by, srand((unsigned) time(NULL)); All these,

for(i=0;i<15;i++)

done[i]=0; /* None of the 16 numbers have been used yet*/

could have been done by, done[HIGH] = { 0 };

for(i=0;i<4;)
  {
    .
    .
    .
     i++;
  }

There's not advantage in taking that incrementation out of the for loop and it is kind of ugly. val=rand()%HIGH; /*generates a number between 0 and 15*/ You are randomizing the order of 16 numbers, not assigning 16 random numbers.

[VernonDozier]

Edited code:
Changed int done[15] to int done [16]

int HIGH=16; /*(Max value of an array element)+1*/
 int done[16]; /* Shows which of the 16 numbers have already been used*/
 int i, j, val;
 time_t secs;
 time(&secs); /*assign 'secs' with the current time*/
 srand((unsigned)secs); /*initialize the seed for rand()*/
 for(i=0;i<15;i++)
  done[i]=0; /* None of the 16 numbers have been used yet*/

  for(i=0;i<4;)
  {
     for(j=0; j<4;)
     {
        val=rand()%HIGH;        /*generates a number between 0 and 15*/
        if(!done[val])  /*if number hasn't been used already*/
       {
         a[i][j]=val;
         done[val]=1; /* mark the particular number as done*/
         j++; /* increment j only if the current array position gets assigned a number*/
       }
     }
     i++;
  }

You still have to fix your initialization of the done[15] element, but the seg fault is gone since you've now set aside 16 elements. As far as the logic goes, it makes sense and it seems like it should work (lines 10 to 23) once you initialize done[15] to 0 initially.

[devnar]

Thanks for the replies VernonDozier and Aia. I'll consider the changes u've suggested and try to correct them. I'll mark the thread as solved.

Thanks a bunch! :)

[devnar]

Finally got it to work! thanks guys! :)