Random number generator only generating one random number

Viewed 235399

I have the following function:

//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);

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?

15 Answers

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 a 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);
    }
}

Edit (see comments): why do we need a lock here?

Basically, Next is going to change the internal state of the Random instance. If we do that at the same time from multiple threads, you could argue "we've just made the outcome even more random", but what we are actually doing is potentially breaking the internal implementation, and we could also start getting the same numbers from different threads, which might be a problem - and might not. The guarantee of what happens internally is the bigger issue, though; since Random does not make any guarantees of thread-safety. Thus there are two valid approaches:

  • Synchronize so that we don't access it at the same time from different threads
  • Use different Random instances per thread

Either can be fine; but mutexing a single instance from multiple callers at the same time is just asking for trouble.

The lock achieves the first (and simpler) of these approaches; however, another approach might be:

private static readonly ThreadLocal<Random> appRandom
     = new ThreadLocal<Random>(() => new Random());

this is then per-thread, so you don't need to synchronize.

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.

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);

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.

Every time you execute

Random random = new Random (15);

It does not matter if you execute it millions of times, you will always use the same seed.

If you use

Random random = new Random ();

You get different random number sequence, if a hacker guesses the seed and your algorithm is related to the security of your system - your algorithm is broken. I you execute mult. In this constructor the seed is specified by the system clock and if several instances are created in a very short period of time (milliseconds) it is possible that they may have the same seed.

If you need safe random numbers you must use the class

System.Security.Cryptography.RNGCryptoServiceProvider

public static int Next(int min, int max)
{
    if(min >= max)
    {
        throw new ArgumentException("Min value is greater or equals than Max value.");
    }
    byte[] intBytes = new byte[4];
    using(RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider())
    {
        rng.GetNonZeroBytes(intBytes);
    }
    return  min +  Math.Abs(BitConverter.ToInt32(intBytes, 0)) % (max - min + 1);
}

Usage:

int randomNumber = Next(1,100);

Why does that happen?

As was answered before, every time you call new Random() you get new copy of Random class initialized with the same clocks (so it returns the same values).

Now, starting from .NET 6, there is easy to use and thread-safe alternative: Random.Shared

In your example you can remove at all function RandomNumber and then use following code (with the same logic, but now it works correct):

byte[] mac = new byte[6];
for (int x = 0; x < 6; ++x)
    mac[x] = (byte)(Random.Shared.Next(0, 255));

You can use code like this:

public static class ThreadSafeRandom
{
    private static readonly Random _global = new Random();
    private static readonly ThreadLocal<Random> _local = new ThreadLocal<Random>(() =>
    {
        int seed;
        lock (_global)
        {
            seed = _global.Next();
        }
        return new Random(seed);
    });

    public static Random Instance => _local.Value;
}

This code can be used as is or via the NuGet package ThreadSafeRandomizer.

Starting from .NET 6, the Random class is now equipped with a static property named Shared:

Provides a thread-safe Random instance that may be used concurrently from any thread.

You could use it like this:

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

Accessing a thread-safe object has a small overhead, so if you are planning to generate millions of random numbers on a single thread as fast as possible, it might be preferable to create a dedicated Random instance instead of relying on the Shared.

I use this:

int randomNumber = int.Parse(Guid.NewGuid().ToString().FirstOrDefault(Char.IsDigit).ToString().Replace("\0", "0"));

Performance: Generating 1 million random number on my PC: 711 ms.

If the Guid not contains any number (i don't know that's possible or not) then 0 will be used as the result.

I solved the problem by using the Rnd() function:

Function RollD6() As UInteger
        RollD6 = (Math.Floor(6 * Rnd())) + 1
        Return RollD6
End Function

When the form loads, I use the Randomize() method to make sure I don't always get the same sequence of random numbers from run to run.

In Visual Basic this works (probably can be translated to C#, if not a DLL reference can be a solution):

Private Function GetRandomInt(ByVal Min As Integer, ByVal Max As Integer) As Integer
     Static Generator As System.Random = New System.Random()
     Return Generator.Next(Min, Max)
End Function

Always get a positive random number.

 var nexnumber = Guid.NewGuid().GetHashCode();
        if (nexnumber < 0)
        {
            nexnumber *= -1;
        }
Related