ChatGPT解决这个技术问题 Extra ChatGPT

Random number generator only generating one random number

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?

using new Random().Next((int)0xFFFF, (int)0xFFFFFF) % 256); doesn't yield any better "random" numbers than .Next(0, 256)
You may find this NuGet package helpful. It provides a static Rand.Next(int, int) method which provides static access to random values without locking or running into the seed re-use problem

M
Mehmet Karadeniz

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.


As a general rule, all static methods should be made thread-safe, since it is hard to guarantee that multiple threads won't call it at the same time. It is not usually necessary to make instance (i.e. non-static) methods thread-safe.
@Florin - there is no difference re "stack based" between the two. Static fields are just as much "external state", and will absolutely be shared between callers. With instances, there is a good chance that different threads have different instances (a common pattern). With statics, it is guaranteed that they all share (not including [ThreadStatic]).
Why can't you use lock(random)?
@Dan if the object is never exposed publicly: you can. The (very theoretical) risk is that some other thread is locking on it in ways you did not expect.
@smiron It's very likely you're simply using the random outside of a lock as well. Locking doesn't prevent all access to what you're locking around - it just makes sure that two lock statements on the same instance will not run concurrently. So lock (syncObject) will only help if all random.Next() calls are also within lock (syncObject). If the scenario you describe does happen even with correct lock usage, it also extremely likely happens in a single-threaded scenario (e.g. Random is subtly broken).
P
Phil

For ease of re-use throughout your application a static class may help.

public static class StaticRandom
{
    private static int seed;

    private static ThreadLocal<Random> threadLocal = new ThreadLocal<Random>
        (() => new Random(Interlocked.Increment(ref seed)));

    static StaticRandom()
    {
        seed = Environment.TickCount;
    }

    public static Random Instance { get { return threadLocal.Value; } }
}

You can use then use static random instance with code such as

StaticRandom.Instance.Next(1, 100);

H
Hans Malherbe

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.


Locks are very cheap when they aren't contested... and even if contested I would expect the "now do something with the number" code to dwarf the cost of the lock in most interesting scenarios.
Agreed, this solves the locking problem, but isn't this still a highly complicated solution to a trivial problem: that you need to write ''two'' lines of code to generate a random number instead of one. Is this really worth it to save reading one simple line of code?
+1 Using an additional global Random instance for getting the seed is a nice idea. Note also that the code can be further simplified using the ThreadLocal<T> class introduced in .NET 4 (as Phil also wrote below).
Given that _local is ThreadStatic, why do you copy it to/from var localBuffer? Is that a performance optimization? That is, is the performance of access to a ThreadStatic variable significantly more expensive than access to a regular variable? (If so, that may nullify the alleged advantage over lock, in typical situations. If not, then the code could be simplified.)
@ToolmakerSteve Yes, the stack is faster that TSS. I'm not worried about the cost compared to locking as locking introduces 100's to 1000's of cycles. The issue with my solution is the branch introduced by the "If" statement potentially costing 100+ cycles due to the flushing of the pipeline and the instruction cache when the branch predictor gets it wrong.
C
Community

My answer from here:

Just reiterating the right solution:

namespace mySpace
{
    public static class Util
    {
        private static rnd = new Random();
        public static int GetRandom()
        {
            return rnd.Next();
        }
    }
}

So you can call:

var i = Util.GetRandom();

all throughout.

If you strictly need a true stateless static method to generate random numbers, you can rely on a Guid.

public static class Util
{
    public static int GetRandom()
    {
        return Guid.NewGuid().GetHashCode();
    }
}

It's going to be a wee bit slower, but can be much more random than Random.Next, at least from my experience.

But not:

new Random(Guid.NewGuid().GetHashCode()).Next();

The unnecessary object creation is going to make it slower especially under a loop.

And never:

new Random().Next();

Not only it's slower (inside a loop), its randomness is... well not really good according to me..


I do not agree with the Guid case. The Random class implements a uniform distribution. Which is not the case in Guid. Guid aim is to be unique not uniformly distributed (and its implementation is most of the time based on some hardware/machine property which is the opposite of ... randomness).
if you cannot prove the uniformity of Guid generation , then it is wrong to use it as random (and the Hash would be another step away from uniformity). Likewise, collisions aren't an issue: uniformity of collision is. Concerning the Guid generation not being on hardware anymore I'm going to RTFM, my bad (any reference?)
There is two understandings of "Random": 1. lack of pattern or 2. lack of pattern following an evolution described by a probability distribution (2 included in 1). Your Guid example is correct in case 1, not in case 2. In opposite: Random class matches case 2 (thus, case 1 too). You can only replace the usage of Random by your Guid+Hash if you are not in case 2. Case 1 is probably enough to answer the Question, and then, your Guid+Hash works fine. But it is not clearly said (ps: this uniform)
@Askolein Just for some test data, I run several batches of both Random and Guid.NewGuid().GetHashCode() through Ent (fourmilab.ch/random) and both are similarly random. new Random(Guid.NewGuid().GetHashCode()) works just as well, as does using a synchronized "master" Random to generate seeds for "child" Randoms.. Of course, it does depend on how your system generates Guids - for my system, they are quite random, and on others it may even be crypto-random. So Windows or MS SQL seems fine nowadays. Mono and/or mobile might be different, though.
@EdB As I have said in comments previously, while Guid (a large number) is meant to be unique, the GetHashCode of the Guid in .NET is derived from its string representation. The output is quite random for my liking.
P
Picrofo Software

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

I'm not one of the down-voters, but note that standard PNRG do serve a genuine need - i.e. to be able to repeatably reproduce a sequence from a known seed. Sometimes the sheer cost of a true cryptographic RNG is too much. And sometimes a crypto RNG is necessary. Horses for courses, so to speak.
According to the documentation this class is thread-safe, so that's something in it's favour.
What is the probability two random strings to be one and the same using that? If the string is just 3 characters I guess this will happen with high probability but what if is 255 characters length is it possible to have the same random string or is guaranteed that this can't happen from the algorithm?
@LyubomirVelchev - It is mathematically impossible to craft a function (or a piece of hardware or even a theoretical construct) that guarantees two independently generated strings of finite length are never the same. It cannot be: there is a finite number of choices. Given n possible strings, there is - and must be - a 1/n probability of two independent strings being the same. (And yes, this implies any cryptographic scheme is not 100% safe; however if the odds of something happening twice during the lifetime of the universe is low enough ... good enough in practice.)
Joma's later answer contains a more complete code snippet based on RNGCryptoServiceProvider. See public static int Next(int min, int max) .... But for performance, modify his code to move the new out of the Next method - see my comment there.
3
3 revs, 3 users 88%

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.


I would move System.Environment.TickCount out of the loop. If it ticks over while you are iterating then you will have two items initialized to the same seed. Another option would be to combine the tickcount an i differently (e.g. System.Environment.TickCount<<8 + i)
If I understand correctly: do you mean, it could happen, that "System.Environment.TickCount + i" could result the SAME value?
EDIT: Of course, no need to have TickCount inside the loop. My bad :).
The default Random() constructor calls Random(Environment.TickCount) anyway
@Alsty - Useful observation - if only creae one global Random generator. However, if you call the default Random() constructor twice during the same tick, you will get two Random generators that each generate the exact same sequence of random numbers. Probably not what you want! The above logic (#2) uses seeds TickCount+0, TickCount+1, etc - so the generators are all different.
J
Joma

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

It does not matter if you execute it millions of times, you will always use the same seed. That's not true unless you specify the seed yourself.
Fixed up. Thanks Exactly as you say LarsTech, if the same seed is always specified, the same sequence of random numbers will always be generated. In my answer I refer to the constructor with parameters if you always use the same seed. The Random class generates only pseudo random numbers. If someone finds out what seed you have used in your algorithm, it can compromise the security or randomness of your algorithm. With the RNGCryptoServiceProvider class, you can safely have random numbers. I already corrected, thank you very much for the correction.
It is excessive to call new RNGCryptoServiceProvider() on every Next. Instead, declare private static RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider(); Then remove the using wrapper; simply call rng.GetNonZeroBytes(intBytes); on that static.
Re "The Random class generates only pseudo random numbers." - ALL software algorithms generate pseudo-random number sequences. True randomness requires hardware based on some physical phenomena that is considered "truly random". OTOH, cryptographic algorithms have been carefully designed (and tested) to improve the statistical distribution of the generated sequence - to avoid brute force attacks that can exploit weaknesses in simpler random generators. Even though overkill for many uses, I agree that this gives a superior statistical distribution.
M
Mark Cilia Vincenti

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.


S
SZL

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.


M
Marztres

There are a lot of solutions, here one: if you want only number erase the letters and the method receives a random and the result length.

public String GenerateRandom(Random oRandom, int iLongitudPin)
{
    String sCharacters = "123456789ABCDEFGHIJKLMNPQRSTUVWXYZ123456789";
    int iLength = sCharacters.Length;
    char cCharacter;
    int iLongitudNuevaCadena = iLongitudPin; 
    String sRandomResult = "";
    for (int i = 0; i < iLongitudNuevaCadena; i++)
    {
        cCharacter = sCharacters[oRandom.Next(iLength)];
        sRandomResult += cCharacter.ToString();
    }
    return (sRandomResult);
}

The basic problem is still the same - you're passing in a Random instance, but you're still expecting the caller to create a shared instance. If the caller creates a new instance each time and the code is executed twice before the clock changes, you'll get the same random number. So this answer still makes assumptions which could be erroneous.
Also, the whole point of having a method to generate random numbers is encapsulation - that the calling method doesn't have to worry about the implementation, it's only interested in getting a random number back
P
Paolo Barone

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.


This question is about C#, not Visual Basic.NET. (Although both are .NET languages, and even though it's possible, but not so trivial, to access VB functions from C#.)
S
SZL

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

D
Deependra Kushwah

Always get a positive random number.

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

This code does not use the Random object as is obvious in the question and GUIDs do not aim to be technically random (as mentioned above).