Why does it appear that my random number generator

Question

I'm working in Microsoft Visual C# 2008 Express.

I found this snippet of code:

    public static int RandomNumber(int min, int max)
    {
        Random random = new Random();

        return random.Next(min, max);
    }

the problem is that I've run it more than 100 times, and it's ALWAYS giving me the same answer when my min = 0 and max = 1. I get 0 every single time. (I created a test function to run it - really - I'm getting 0 each time). I'm having a hard time believing that's a coincidence... is there something else I can do to examine or test this? (I did re-run the test with min = 0 and max = 10 and the first 50ish times, the result was always "5", the 2nd 50ish times, the result was always "9".

?? I need something a little more consistently random...

-Adeena

Answer 1

You're always getting 0 because Random.Next returns integers. You need to call Random.NextDouble, which will return a number between 0 and 1. Also, you should reuse your Random instance, like this:

[ThreadStatic]
static Random random;
public static Random Random { 
    get {
        if (random == null) random = new Random();
        return random;
    }
}
public static int RandomInteger(int min, int max)
{
    return Random.Next(min, max);
}
public static double RandomDouble() //Between 0 and 1
{ 
    return Random.NextDouble();
} 

If you want cryptographically secure random numbers, use the RNGCryptoServiceProvider class; see this article

EDIT: Thread safety

Answer 2

You're misunderstanding the line "random.Next(min, max)". "min" is in the place of the lowest number allowed to be generated randomly. While "max" is in the place of the lowest number NOT allowed to be generated it is not in the place of the largest number allowed to be drawn. So when the line is random.Next(0, 1) you are basically only allowing 0 to be drawn.

Answer 3

This is an addendum to any answers, as the answer to this specific question is the bounds should be (0, 2) not (0, 1).

However, if you want to use a static wrapper method, then you must remember that Random is not thread-safe, so you either need to provide your own synchronization mechanism or provide a per-thread instance. Here is a largely non-blocking implementation which uses one generator to seed each per-thread generator:

public static class ThreadSafeRandom
{
    private static readonly Random seed = new Random();

    [ThreadStatic]
    private static Random random;

    public static int Next(int min, int max)
    {
        if (random == null)
        {
            lock (seed)
            {
                random = new Random(seed.Next());
            }
        }

        return random.Next(min, max);
    }

    // etc. for other members
}

Answer 4

I found a very simple, but effective way to generate random numbers by just taking the last two digits of the current datetime milliseconds:

   int seed = Convert.ToInt32(DateTime.Now.Millisecond.ToString().Substring(1, 2));
   int cnr = new Random(seed).Next(100);

It is crude, but it works! :-)

Of course it would statistically generate the same number every hundred times. Alternatively, you could take all three digits or concatenate with other datetime values like seconds or so.

Answer 5

Besides the 0-1 issue already noted in other answers, your problem is a real one when you're looking for a 0-10 range and get identical results 50 times in a row.

new Random() is supposed to return a random number with a seed initialized from the timer (current second), but apparently you're calling this code 50 times a second. MSDN suggests: "To improve performance, create one Random to generate many random numbers over time, instead of repeatedly creating a new Random to generate one random number.". If you create your random generator once outside the method, that should fix your "non-randomness" problem as well as improving performance.

Also consider this post for a better pseudo-random number generator than the system-supplied one, if you need "higher quality" pseudo-random numbers.

Answer 6

Don't create a wrapper method for Next. It wastes cycles creating a new instance of the Random class. Just use the same one!

Random myRand = new Random();

for(int i = 0; i < 10; i++)
{
    Console.WriteLine(myRand.Next(0, 10).ToString());
}

That should give you ten random values.

As has been said--Random is pseudo-random (as all implementations are), and if you create 100 instances with the same seed, you'll get 100 instances of the same results. Make sure that you're reusing the class.

Also, as folks have said, beware that MinValue is inclusive and MaxValue is exclusive. For what you want, do myRand.Next(0, 2).