3

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

I created a method that returns me a random number:

public static int SelectRandomMachine(int max)
{
int seed = (int)DateTime.Now.Ticks;
Random rndNumber = new Random(seed);
int randMachine = rndNumber.Next(0, max);
return randMachine;
}

if I call the method two times, currently it's return me the same random number:

randM1 = SelectRandomMachine(maxNumber);
randM2 = SelectRandomMachine(maxNumber);

any suggestion would be highly appreciated.

Community
  • 1
  • 1
MKS
  • 736
  • 2
  • 14
  • 33
  • 1
    You don't need to re-seed the generator every time. That is likely a good part of your problem. Make the Random instance a class member and only initialize it once. – Joe Sep 28 '11 at 16:44
  • Sometimes I wonder... This seems incredibly hard to grasp for most people, maybe seed-before-querying random number generator pattern isn't the best one? How many questions like these would have been avoided with an automatically seeded, global (or thread local) random number generator... – Blindy Sep 28 '11 at 16:46
  • public int GetRandomNumber {return 4;}//I randomly chose 4. – Jeremy Holovacs Sep 28 '11 at 16:47
  • @Blindy: But the problem with a global or thread local RNG is that you can only have one (or one per thread). It's not uncommon to want different parts of your program to be using different random sequences. This is especially true if you're using a 3rd party library that uses random numbers for something (possibly with a custom seed) and you want your code also to generate a sequence based on a particular seed. I agree, though, that many people seem to have trouble grasping the "only seed it once" concept. – Jim Mischel Sep 28 '11 at 16:50
  • @Jim, that can be solved by also providing the object version of `Random` if needed, but the learning curve would be a lot less steep with a properly initialized default. It's not *that* common to want different rng's, in fact I only ever used that once, for a perlin noise generating module. – Blindy Sep 28 '11 at 16:58

5 Answers5

6

Hint look at this line:

int seed = (int)DateTime.Now.Ticks;

If you execute that line twice in quick succession, what do you think the values will be?

For example:

int seed1 = (int)DateTime.Now.Ticks;
int seed2 = (int)DateTime.Now.Ticks;

// Write it out *after* executing; console output can take a while
Console.WriteLine(seed1);
Console.WriteLine(seed2);

See my article on randomness for solutions and more information.

EDIT: Here's a quick and dirty example of the lack of thread safety causing problems:

using System.Collections.Generic;
using System.Threading;

class Program
{
    const int Iterations = 1000000;
    static readonly Random rng = new Random();

    static void Main(string[] args)
    {
        List<Thread> threads = new List<Thread>();
        for (int i = 0;  i < 8; i++)
        {
            Thread t = new Thread(ExerciseRandom);
            threads.Add(t);
            t.Start();
        }
        foreach (Thread t in threads)
        {
            t.Join();
        }
        Console.WriteLine(rng.Next());
        Console.WriteLine(rng.Next());
        Console.WriteLine(rng.Next());
    }

    static void ExerciseRandom()
    {
        for (int i = 0; i < Iterations; i++)
        {
            rng.Next();
        }
    }
}

Output on my box:

0
0
0
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
2

You need to create a single instance of a Random object and call it several times.

Since you are basing your seed on time (number of ticks), quick calls in succession will end up generating the same seed value, hence the pseudo-random number generator will generate the same sequence.

Oded
  • 489,969
  • 99
  • 883
  • 1,009
1

Make the Random instance static:

static Random rndNumber = new Random((int)DateTime.Now.Ticks);

public static int SelectRandomMachine(int max)
{
   int randMachine = rndNumber.Next(0, max);
   return randMachine;
}
Simon Mourier
  • 132,049
  • 21
  • 248
  • 298
  • Generally speaking this is a bad idea, unless you know you'll only ever execute this code in a single thread. `Random` is *not* thread-safe. – Jon Skeet Sep 28 '11 at 16:45
  • I am pretty sure it uses current date as seed with default constructor – Ilia G Sep 28 '11 at 16:46
  • @Jon Skeet - Sure, I was just answering the question, Random is anyway the poor man's randomizer. – Simon Mourier Sep 28 '11 at 16:47
  • @liho1eye I think that's correct. That means if the calls are too close together they will get seeded with the same number. – N_A Sep 28 '11 at 16:47
  • 1
    @SimonMourier: Well, you were suggesting a course of action - and one which can cause problems if used in a multi-threaded environment, without warning the OP of that possibility. It would be far from unreasonable for the OP to have taken your suggestion, created a static field in (say) an ASP.NET application, and ended up with problems. – Jon Skeet Sep 28 '11 at 16:48
  • @Jon, what's the worst that can happen though, randomize the sequence more due to partial overwrites from multiple threads? – Blindy Sep 28 '11 at 16:50
  • @Blindy: No, it can return 0 for every call to `Next()` thereafter IIRC. I've seen that happen - it's a pretty grim failure mode, and one which is hard to reproduce. – Jon Skeet Sep 28 '11 at 16:50
  • wth I had always assumed it was just a dumb `return seed=(a*seed+b)%randmax` for some `a` and `b`.. – Blindy Sep 28 '11 at 16:53
  • Note that, prior to .NET 4.0, calling `Random(int32)` with a value of `-MaxInt` would throw an exception. In your code, if `(int)DateTime.Now.Ticks` returned `-MaxInt`, this code would fail. – Jim Mischel Sep 28 '11 at 16:54
  • @liho1eye: The default constructor uses `Environment.TickCount` as the seed, not the current date. – Jim Mischel Sep 28 '11 at 16:56
  • @Blindy - Actual implementation is here: http://www.koders.com/csharp/fidEE33719760CE27B9092A158E50434C550858EDD3.aspx – Simon Mourier Sep 28 '11 at 16:57
  • @Jim Mischel that seems irrelevant to my point. Calling `new Random((int)DateTime.Now.Ticks);` is not any better than just `new Random();` – Ilia G Sep 28 '11 at 17:11
  • @Blindy: I've added sample code you can use to test this. (I used 8 threads as my machine has 8 cores; adjust to taste.) – Jon Skeet Sep 28 '11 at 17:12
0

You need to have a private static Random() and refer to it to get the random numbers, Jon and Oded are right, you can't call in quick succession.

private static Random _rnd = new Random();
public static int SelectRandomMachine(int max)
{
//int seed = (int)DateTime.Now.Ticks;
//Random rndNumber = new Random(seed);
int randMachine = _rnd.Next(0, max);
return randMachine;
}
Matt
  • 3,638
  • 2
  • 26
  • 33
0

Random number generators generate numbers using an algorithm. If you seed the algorithm with the same number, it will generate the same sequence of numbers. This means if you seed the rand with a number each time and then pull the first number of the sequence you will always get the same number. You need to seed the rand once and then use Next() repeatedly on the same Rand object.

N_A
  • 19,799
  • 4
  • 52
  • 98