Random number generator not working the way I had planned (C#)

Question

This is a very strange problem.

Here is my code:

    //Function to get random number
    public static int RandomNumber(int min, int max)
    {
        Random random = new Random();
        return random.Next(min, max);
    }

How I call it:

            byte[] mac = new byte[6];
            for (int x = 0; x < 6; ++x)
                mac[x] = (byte)(Misc.RandomNumber((int)0xFFFF, (int)0xFFFFFF) % 256);

Problem:

If I step that loop with the debugger during runtime I get different values(which is what I want). However, if I put a breakpoint two lines below that code,all members of the "mac" array have equal value.

Why does that happen?

Answer 1

Every time you do new Random() it is initialized using the clock. This means that in a tight loop you get the same value lots of times. You should keep a single Random instance and keep using Next on the same instance.

//Function to get random number
private static readonly Random random = new Random();
private static readonly object syncLock = new object();
public static int RandomNumber(int min, int max)
{
    lock(syncLock) { // synchronize
        return random.Next(min, max);
    }
}

Answer 2

I would rather use the following class to generate random numbers:

byte[] random;
System.Security.Cryptography.RNGCryptoServiceProvider prov = new   System.Security.Cryptography.RNGCryptoServiceProvider();
prov.GetBytes(random);

Answer 3

1) As Marc Gravell said, try to use ONE random-generator. It's always cool to add this to the constructor: System.Environment.TickCount.

2) One tip. Let's say you want to create 100 objects and suppose each of them should have its-own random-generator (handy if you calculate LOADS of random numbers in a very short period of time). If you would do this in a loop (generation of 100 objects), you could do this like that (to assure fully-randomness):

int inMyRandSeed;

for(int i=0;i<100;i++)
{
   inMyRandSeed = System.Environment.TickCount + i;
   .
   .
   .
   myNewObject = new MyNewObject(inMyRandSeed);  
   .
   .
   .
}

// Usage: Random m_rndGen = new Random(inMyRandSeed);

Cheers.

Answer 4

Mark's solution can be quite expensive since it needs to synchronize everytime.

We can get around the need for synchronization by using the thread-specific storage pattern:

public class RandomNumber : IRandomNumber
{
    private static readonly Random Global = new Random();
    [ThreadStatic] private static Random _local;

    public int Next(int max)
    {
        var localBuffer = _local;
        if (localBuffer == null) 
        {
            int seed;
            lock(Global) seed = Global.Next();
            localBuffer = new Random(seed);
            _local = localBuffer;
        }
        return localBuffer.Next(max);
    }
}

Measure the two implementations and you should see a significant difference.