C# Random Numbers Aren't Being "Random"

C# Random Numbers aren't being random

My guess is that randomNumber creates a new instance of Random each time... which in turn creates a new pseudo-random number generator based on the current time... which doesn't change as often as you might think.

Don't do that. Use the same instance of Random repeatedly... but don't "fix" it by creating a static Random variable. That won't work well either in the long term, as Random isn't thread-safe. It will all look fine in testing, then you'll mysteriously get all zeroes back after you happen to get unlucky with concurrency :(

Fortunately it's not too hard to get something working using thread-locals, particularly if you're on .NET 4. You end up with a new instance of Random per thread.

I've written an article on this very topic which you may find useful, including this code:

using System;
using System.Threading;

public static class RandomProvider
{
private static int seed = Environment.TickCount;

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

public static Random GetThreadRandom()
{
return randomWrapper.Value;
}
}

If you change your new Random() call to RandomProvider.GetThreadRandom() that will probably do everything you need (again, assuming .NET 4). That doesn't address testability, but one step at a time...

Random numbers don't seem very random

This is similar to the Birthday Problem. Given a group of n people, What is the probability that two share the same birthday1? It's higher than you'd think.

In your case, what are the odds that randomly picking a number between 0 and 1,073,741,823 n times will give you a duplicate?

One approximation from the link above is 1-exp(-(n*n)/(2*d)). If n=40,000 that equates to about a 52.5% probability that a duplicate is chosen, so seeing duplicates after 40,000 picks on average seems reasonable.


1assuming that birthdays are uniformly distributed universally, which is not the case in reality but is "close enough" and makes the math easier

Getting a random number that isn't the last one?

You have two problems here. First your list is re-initialized every time you call the GetRandomTip() method so it will always be able to output from every tip possible, and ignore the call to RemoveAt(). What you can do is to only re-fill the list when it has 0 elements. This way you're sure all the tips will be shown once before resting the list.

The second one is that you shouldn't re-initialize your Random object every time. You might want to refer to this post for more info.

Here is your code, modified. It should give better results.

private class Test
{
readonly Random Rnd = new Random();
private List<string> _titles = new List<string>(); // Init the list with 0 elements, it will be filled-in on the first call to `GetRandomTip`

private string GetRandomTip()
{
// fill the list if it's empty
if(_titles.Count == 0)
{
_titles = new List<string>
{
"You can copy the result by clicking over it",
"Remember to press Ctrl + Z if you messed up",
"Check web.com to update the app"
};
}

int index = Rnd.Next(0, _titles.Count);
string randomString = _titles[index];
_titles.RemoveAt(index);

return randomString;
}
}

Random numbers generated in separate functions aren't so random

Ways out of this hole:

  • Pass a single Random object around as needed.

  • Make the Random object a static public field of a class visible to everything that needs it. Note that Random is not thread safe. If you need it to be thread safe then either put locks around it, or make it thread local and allocate one for each thread.

  • Use crypto strength randomness. It doesn't have this problem.

  • Write your own pseudo random number generator that has better behaviour than the built-in one.

  • Use another source of randomness entirely, like downloading a blob from random.org or some such thing.

A now-deleted answer suggests using NewGuid as a source of randomness. Do not do this. Guids are guaranteed to be unique; they are not guaranteed to be random. In particular, it is perfectly legal for a guid generator to generate sequential unique guids. That NewGuid does not actually do so is not part of its contract. Use guids for what it says on the box: the box says "globally unique identifier", not "randomly generated identifier".

A commenter suggests using a hash code as a source of randomness. Do not do this. Hash codes are guaranteed to be randomly distributed only so far as they need to be to give a good distribution in a hash table. In particular, we have no guarantee whatsoever that hash functions were designed to ensure that their low order bits are well distributed.

Use only sources of randomness or pseudo-randomness that were designed by experts to produce randomness. Generating randomness is a difficult engineering problem. Just because you think you can't predict a particular outcome does not make that outcome a suitable source of entropy for a random number generator.

Random number generator only generating one random number

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.

Random encounter not so random

You're creating a new Random for each single value you need.

Try creating a unique Random object and calling the .Next() function multiple times.

public Color getRandomColor()
{
Random rand = new Random();

Color1 = rand.Next(rand.Next(0, 100), rand.Next(200, 255));
Color2 = rand.Next(rand.Next(0, 100), rand.Next(200, 255));
Color3 = rand.Next(rand.Next(0, 100), rand.Next(200, 255));
Color color = Color.FromArgb(Color1, Color2, Color3);
Console.WriteLine("R: " + Color1 + " G: " + Color2 + " B: " + Color3 + " = " + color.Name);
return color;
}

Taken from MSDN documentation on Random object :

By default, the parameterless constructor of the Random class uses the system clock to generate its seed value, while its parameterized constructor can take an Int32 value based on the number of ticks in the current time. However, because the clock has finite resolution, using the parameterless constructor to create different Random objects in close succession creates random number generators that produce identical sequences of random numbers

Why does new Random().Next() not return random numbers like random.Next()?

Random class initializes with seed on construction - this seed will be used to produce unique sequence of numbers. In case if seed is not specified - Environment.TickCount is used as seed. In your case, sequence is constructed so fast, that Random class gets same seed in every constructed instance. So all items return same value, and not randomly sorted.

First working line, on the other hand, have single Random class instance for every enumerable item. So calling .Next() produces next random number, although seed is the same, and sequence gets randomly sorted.

Speaking of generating random sequences without duplicates - look at Fisher-Yates shuffle. Your example with Random() has side effects - executing every loop iteration changes internal state of Random() class instance, which can lead to undesired behavior - for example, if sorting algorithm relies on fact that each sequence item returns same number during sort (this is not the case with .net, but who knows - internal implementation may change in future).

Generating random numbers without repeating.C#

Check each number that you generate against the previous numbers:

List<int> listNumbers = new List<int>();
int number;
for (int i = 0; i < 6; i++)
{
do {
number = rand.Next(1, 49);
} while (listNumbers.Contains(number));
listNumbers.Add(number);
}

Another approach is to create a list of possible numbers, and remove numbers that you pick from the list:

List<int> possible = Enumerable.Range(1, 48).ToList();
List<int> listNumbers = new List<int>();
for (int i = 0; i < 6; i++)
{
int index = rand.Next(0, possible.Count);
listNumbers.Add(possible[index]);
possible.RemoveAt(index);
}


Related Topics



Leave a reply



Submit