Random numbers in C aren't random

Question

Here's the code for my program. it's supposed to get two numbers from user then generate random numbers using rand function. but when I compile it, the numbers are not random.

I also wrote this in java, and that one works perfectly.

C version:

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

int cmpfunc();

int main()
{
   puts("How many numbers do you want to generate?");
   int n;
   scanf("%d", &n);
   int numbers[n];

   puts("What should be the largest number possible?");
   int m, i;
   scanf("%d", &m);
   int result[m];

   for(i=0;i<m;i++)
       numbers[i] = i + 1;

       srand(time(NULL));
   for(i=0;i<n;i++)
   {
       int r = rand() % m;

       result[i] = numbers[r];
       numbers[r] = numbers[m - 1];
       m--;

   }

   qsort(result, n, sizeof(int), cmpfunc); //sorting the result array

   for(i=0;i<n;i++)
   {
       printf("%d\n", result[i]);
   }

    getch();
    return 0;
}

int cmpfunc (const void * a, const void * b)
{
   return ( *(int*)a - *(int*)b );
}

EDIT: srand was inside the loop, people said it's not supposed to be called more than once. so i took it out but that wasn't helpful.

Java version:

import java.util.*;

public class RandomNumberGenerator {

    public static void main(String[] args) {

        Scanner in = new Scanner(System.in);

        System.out.println("How many numbers do you want to generate?");
        int n = in.nextInt();

        System.out.println("What should be the largest number possible?");
        int m = in.nextInt();

        int numbers[] = new int[m];
        for(int i=0;i<numbers.length;i++)
            numbers[i] = i + 1;

        int result[] = new int[n];
        for(int i=0;i<result.length;i++)
        {
            // make a random index between 0 and n - 1
            int r = (int) (Math.random() * m );

            result[i] = numbers[r];
            // move the last element into the random location
            numbers[r] = numbers[m - 1];
            m--;
        }

        System.out.println("Do you want the result to be sorted? (1 for Yes, others for No)");
        int ans = in.nextInt();

        if(ans==1)
            Arrays.sort(result);

          System.out.println("Result:");
          for (int r : result)
             System.out.println(r);

          pressAnyKeyToContinue();

    }

    private static void pressAnyKeyToContinue()
     { 
            System.out.println("Press Enter to continue.");
            try
            {
                System.in.read();
            }  
            catch(Exception e)
            {}
     }

}

edit 2:

c output: (is not random, sorted)

java output: (is random, sorted)

edit 3:

as i stated in comments values are : n= 55 m= 9808

also i'm using codeblocks so header files are not really an issue. they are automatically loaded by codeblocks.

edit 4:

I'm really impressed by what people suggest. qsort doesn't change a thing in generating random numbers. it just sorts the array. so even if i remove it, it doesn't change nothing.

In an array of random numbers we would expect that the difference between any two adjacent elements would also be random. This property is preserved under sorting. I sort the random numbers in both the C and Java could to check this property. As you can see, in the C code the differences between adjacent elements are all the same. This leads me to believe that the C code is not generating random numbers, but that the Java code is.

edit 5:

and i'm sure there should be no difference in algorithm between java and c.

edit 6:

I'm sorry that i'm not a professional. i'm just trying to make a C version of the Java program i have. A fixed code would be very appreciated.

****edit final:** Finally got it working. arrays "numbers" and "result" storing n and m respectively should store m and n instead. that's all. in addition to that i changed their names with what people suggested. some folks told me to switch the variables m and n with array dims. that just wasn't the case. here is the fixed code for anyone interested:**

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

int cmpfunc();

int main()
{
   puts("How many numbers do you want to generate?");
   int  num_numbers;
   scanf("%d", &num_numbers);

   puts("What should be the largest number possible?");
   int num_maximum, i;
   scanf("%d", & num_maximum);


   int numbers[num_maximum];
   int result[num_numbers];

   for(i=0;i<num_maximum;i++) // making
       numbers[i] = i + 1;

       srand(time(NULL)); // seeding rand

   for(i=0;i<num_numbers;i++)
   {
       int r = rand() %  num_maximum; // generates random number between 0 and m-1

       result[i] = numbers[r]; //

       numbers[r] = numbers[ num_maximum - 1];
        num_maximum--;

   }

   puts("Do you want the result to be sorted?");
   int ans;
   scanf("%d", &ans);

   if (ans==1)
   qsort(result,  num_numbers, sizeof(int), cmpfunc); //sorting the result array

   for(i=0;i< num_numbers;i++)
   {
       printf("%d\n", result[i]);
   }

    getch();
    return 0;
}

int cmpfunc (const void * a, const void * b)
{
   return ( *(int*)a - *(int*)b );
}

Answer 1

srand(time(NULL)); is used to seed the random number generator. It needs to be called only once. You need to move that out of the for loop.

If you seed every time, you're going to end up with the same number being produced by rand().

Also, time() is prototyped in time.h.

Next, as mentioned by @Blastfurnace, you should be passing m instead of n as the second argument to qsort().

Then,

for(i=0;i<m;i++)
   numbers[i] = i + 1;

may (will) lead to UB by producing out-of-bound access for numbers array.

Answer 2

In your Java code:

int numbers[] = new int[m];
int result[] = new int[n];

In your C code:

int numbers[n];
int result[m];

The size of the arrays are reversed. The code seems to work when I match the definition on the C code to that of your Java code.

Answer 3

Question says:

n= 55 m= 9808

However the code includes:

int numbers[n];

for(i=0;i<m;i++)
   numbers[i] = i + 1;

After i passes 55 then this is a buffer overflow, causing undefined behaviour.

The second loop has the same bug (n and m swapped); Another bug is that the wrong dimension is passed to qsort.

---

To fix these errors, stop using m and n in the code after the point where you declared the arrays. Instead, use the dimension of the array. One way is to write:

#define dimof(ARR) ( sizeof(ARR) / sizeof (ARR)[0] )

and then you can use it:

for (int i = 0; i < dimof(numbers); ++i)
    numbers[i] = i + 1;

This way there is no chance of mixing up which variable goes with which array.

You'll have to do the same thing for the second loop, and the qsort dimension.

Answer 4

Because it's easier than pointing to each error, here's a transliteration of the Java code into C with some more meaningful variable names (Horstmann should know better) but without the sorting code (which was becoming a distraction):

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

int main( void )
{
  int sampleSize, valueRange;
  printf( "How many samples do you want to take: " );
  fflush( stdout );
  scanf( "%d", &sampleSize );

  printf( "What is the range of values, starting from 1: " );
  fflush( stdout );
  scanf( "%d", &valueRange );

  /**
   * The numbers array contains your source values, ranging
   * from 1 to valueRange.  The result array contains a random,
   * non-repeating sample of the numbers array.
   */
  int numbers[valueRange];
  int result[sampleSize];

  for ( int i = 0; i < valueRange; i++ )
    numbers[i] = i + 1;

  srand( time( NULL ));

  for ( int i = 0; i < sampleSize; i++ )
  {
    int sampleIndex = rand() % valueRange;
    result[i] = numbers[sampleIndex];
    numbers[sampleIndex] = numbers[ valueRange - 1 ];
    valueRange--;
  }

  for ( int i = 0; i < sampleSize; i++ )
    printf( "result[%4d] = %d\n", i, result[i] );

  return 0;
}

Sample sessions:

[fbgo448@n9dvap997]~/prototypes/rand: ./stackrand
How many samples do you want to take: 10
What is the range of values, starting from 1: 10
result[   0] = 5
result[   1] = 6
result[   2] = 7
result[   3] = 4
result[   4] = 10
result[   5] = 3
result[   6] = 2
result[   7] = 8
result[   8] = 1
result[   9] = 9
[fbgo448@n9dvap997]~/prototypes/rand: ./stackrand
How many samples do you want to take: 10
What is the range of values, starting from 1: 10
result[   0] = 9
result[   1] = 3
result[   2] = 7
result[   3] = 1
result[   4] = 4
result[   5] = 8
result[   6] = 2
result[   7] = 5
result[   8] = 6
result[   9] = 10
[fbgo448@n9dvap997]~/prototypes/rand: ./stackrand
How many samples do you want to take: 10
What is the range of values, starting from 1: 100
result[   0] = 28
result[   1] = 90
result[   2] = 100
result[   3] = 4
result[   4] = 15
result[   5] = 76
result[   6] = 3
result[   7] = 60
result[   8] = 65
result[   9] = 21

Be careful when using variable-length arrays like this; if you put in large enough values for your range or sample size you'll overrun your stack space and get a runtime error. It would be safer to allocate the space using malloc or calloc, but for small ranges and sample sizes this is definitely easier and less hassle.

Also, you should add some code to make sure your sample size is always less than your value range. or Bad Things™ will happen.

Answer 5

the following code may be what your looking for:

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

int cmpfunc (const void * a, const void * b);

int main()
{
    puts("How many numbers do you want to generate?");
    int n;
    scanf("%d", &n);

    puts("What should be the largest number possible?");
    int m;
    scanf("%d", &m);
    int result[n];

    int i;

    srand(time(NULL));

    for(i=0;i<n;i++)
    {
        result[i] = rand() % m;
    }

    qsort(result, n, sizeof(int), cmpfunc); //sorting the result array

    for(i=0;i<n;i++)
    {
        printf("%d\n", result[i]);
    }

    getchar();
    return 0;
} // end function: main


int cmpfunc (const void * a, const void * b)
{
    return ( *(int*)a - *(int*)b );
} // end function: cmpfunc