75

This is really weird, and I cannot see why this is happening. In the foreach cycle, I am iterating through a class A collection, and for each class, I call the Count() method, where r1 and r2 numbers are generated from range [-1,1]. The problem is that Random.Next returns the same "random" numbers for each instance. When the results for the first instance are 0 and -1, the same ones will be returned from following instances. Please, could you tell me why this is happening? Also, I cannot get different results in each class A instance. This is the code:

class a
{
 Random rnd = new Random();
 private void Count()
 {
  int r1 = rnd.Next(-1, 1);
  int r2 = rnd.Next(-1, 1);
 }
}
class b
{
 List<a> listofA=new list<a>();
 foreach (a ACLASS in listofA)
 {
  ACLASS.Count();
 }
}
tvanfosson
  • 524,688
  • 99
  • 697
  • 795
Thomas
  • 2,575
  • 9
  • 31
  • 41

4 Answers4

142

The problem is that you are creating instances of the Random class too close in time.

When you create a Random object, it's seeded with a value from the system clock. If you create Random instances too close in time, they will all be seeded with the same random sequence.

Create a single Random object and pass its reference to the constructor when you create instances of the "a" class, instead of creating one Random object for each "a" instance.

Svante
  • 50,694
  • 11
  • 78
  • 122
Guffa
  • 687,336
  • 108
  • 737
  • 1,005
  • 4
    I forgot as well, I used to have the same issue back in the day making Bingo Cartons for a club, and back then, I used the worst trick ever known to man: Pausing the thread for 2 Ms. Inexperienced and crazy... Crazy enough, I have a class that creates random names with a static Random declaration on top of everything. – Luis Robles Jun 26 '13 at 04:25
16

Use a single, static Random number generator for all instances of the class.

class a
{
  private static Random rnd;
  static a() {
      rnd = new Random();
  }
  private void Count()
  {
    int r1 = rnd.Next(-1, 2);
    int r2 = rnd.Next(-1, 2);
  }
}

Note the change to give you numbers in the range -1,1 rather than -1,0

tvanfosson
  • 524,688
  • 99
  • 697
  • 795
  • i think with this range it can also return 1 and they only want 0 and -1? – Lucas Oct 31 '09 at 18:38
  • @Svante edited the question to make the interval open to match the code sample. The original question specified a closed interval, though not using precise language. I think the code was incorrect and am going to restore the question to specify a closed interval. – tvanfosson Oct 31 '09 at 18:57
  • @tvanfosson Can you tell me why this works? I don't understand how making this statics gives you randomness. I know it works, but not why. They are still very close in time. Thanks. – johnny Nov 08 '16 at 18:06
  • 3
    @johnny when it's created as an instance variable, the constructor for `Random` is executed twice in quick succession. Since it uses the current time as the seed, this can result in both random number generators using the same sequence of pseudorandom numbers and thus they both return the same values in lock step. Using a single random number generator, initialized once in the static constructor for the class, returns different numbers from the single pseudorandom sequence giving the behavior that you would expect from a random number generator. – tvanfosson Nov 08 '16 at 18:17
9

You're creating new instances of Random very close together (your loop is very tight) so each instance is effectively using the same seed value.

A better approach would be to create one instance and pass that to your Count method.

You probably know this next bit, but I'll include it here for completeness:

The MSDN has the details on this, but basically your problem is the Random.Next method you're using generates:

A 32-bit signed integer greater than or equal to minValue and less than maxValue; that is, the range of return values includes minValue but not maxValue. If minValue equals maxValue, minValue is returned.

because of this your calls will return -1 or 0.

ChrisF
  • 134,786
  • 31
  • 255
  • 325
5

You include a random instance for each A instance. It sounds like they're all getting the same default seed value. You probably want to make a static random for all A instances and use it repeatedly, or alternatively provide a seed value to the Random() instance in the A constructor.

Jherico
  • 28,584
  • 8
  • 61
  • 87