3

I'm using a method to create two new int arrays with random numbers, but the two array contains exactly the same numbers. Why is this happening?

    static void Main(string[] args)
    {
        int[] Foo1= Foo(1000);
        int[] Foo2= Foo(1000);
    }
    static int[] Foo(int length)
    {
        int[] Array = new int[length];
        Random r = new Random();
        for (int i = 0; i < length; i++)
        {          
            Array[i]=r.Next(1, 101);   
        }
        //Thread.Sleep(6);
        return Array;
    }
George Stocker
  • 57,289
  • 29
  • 176
  • 237
Dominik Antal
  • 3,281
  • 4
  • 34
  • 50
  • 6
    Good grief, there's a `[weird-behaviour]` tag? – BoltClock Feb 11 '11 at 13:31
  • With 2 followers... @BoltClock is having all the fun itseems...;) – Sachin Shanbhag Feb 11 '11 at 13:31
  • 1
    @BoltClock Not for long. – C. Ross Feb 11 '11 at 13:32
  • @George Stocker: That isn't a good edit; the title no longer reflects the OP's original problem - it is half-way to *answering* the OP's question. – Ani Feb 11 '11 at 13:48
  • @Ani I think you should re-read the OPs original revision; I'm only using words he used in his original revision. Also, if you believe somehow 'weird behavior' describes his problem better, then edit it to say so, or take the problem to [Meta](http://meta.stackoverflow.com) and open a question about it there. The community will tell you in short order if my edit was 'not a good edit'. – George Stocker Feb 11 '11 at 13:53
  • @George: Ok, it's just the word "seeding" that I find too 'knowing'. Btw, it was just a comment; if I really thought it was horrible; I would have edited it myself, but I thought it would be better to mention it than mess around with someone else's edit. There's no 'problem' to take to meta here; so don't take offence :). – Ani Feb 11 '11 at 13:58
  • @Ani No offense taken. I see Meta as a check on the 'high rep' users' actions. If one of us takes an action that rubs you the wrong way, the natural place to talk about it is on meta. – George Stocker Feb 11 '11 at 14:00
  • @George: I haven't been rubbed the wrong way! It's just an edit! :) – Ani Feb 11 '11 at 14:01
  • 1
    @George: Can we please delete this noise? – Ani Feb 11 '11 at 14:02

6 Answers6

6

You're not seeding Random but you're likely using it close enough between calls that the default seed is the same in both cases :

The default seed value is derived from the system clock and has finite resolution. As a result, different Random objects that are created in close succession by a call to the default constructor will have identical default seed values and, therefore, will produce identical sets of random numbers. This problem can be avoided by using a single Random object to generate all random numbers. You can also work around it by modifying the seed value returned by the system clock and then explicitly providing this new seed value to the Random(Int32) constructor. For more information, see the Random(Int32) constructor.

MartW
  • 12,348
  • 3
  • 44
  • 68
5

it's because your random is initialized twice, with a too small time difference to have a different seed.

try this :

static void Main(string[] args)
{
    Random r = new Random();
    int[] Foo1= Foo(1000,r);
    int[] Foo2= Foo(1000,r);
}
static int[] Foo(int length, Random r)
{
    int[] Array = new int[length];

    for (int i = 0; i < length; i++)
    {          
        Array[i]=r.Next(1, 101);   
    }
    //Thread.Sleep(6);
    return Array;
}
Steve B
  • 36,818
  • 21
  • 101
  • 174
  • 3
    I would advise *against* using a static variable like this... because Random isn't thread-safe, whereas static members typically should be. Instead, create an instance of `Random` in Main and pass it into the `Foo` method. – Jon Skeet Feb 11 '11 at 13:40
  • @Jon, I updated my code to follow your advice. thx – Steve B Feb 11 '11 at 13:49
  • Cool. That's a lot nicer IMO. It also demonstrates the method's dependencies more elegantly :) – Jon Skeet Feb 11 '11 at 13:55
5

The other answers around "you're using two instances of Random with the same seed" are correct. However, they used to use a static variable to refer to an instance of Random. That could cause problems if you try to use it from multiple threads, because Random isn't thread-safe.

There are various workarounds for this (such as creating one instance of Random per thread, with a static method to access it safely) - I've written quite a long article on this topic which you may find useful.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
3

That is because of the Random class which are not really random numbers, but rather pseudo randoms. So each time a new Random instance is initialized in a short timespan, it begins with the same number again. If the timespan is bigger, you will have a different seed, so it would work. I would suggest to declare the Random instance in Main and hand it over to the Foo method, so it would look like this:

static void Main(string[] args)
{
    Random r = new Random();
    int[] Foo1= Foo(1000, r);
    int[] Foo2= Foo(1000, r);
}
static int[] Foo(int length, Random r)
{
    int[] Array = new int[length];
    for (int i = 0; i < length; i++)
    {          
        Array[i]=r.Next(1, 101);   
    }
    //Thread.Sleep(6);
    return Array;
}

Edit: I modified the code above after Jon Skeet's advice. Now the problem mentioned by him should be gone.

Sören
  • 2,661
  • 2
  • 19
  • 22
3

you can use global Random object or you can use a good seed like

  new Random(Guid.NewGuid().GetHashCode());
Kris Ivanov
  • 10,476
  • 1
  • 24
  • 35
1

Because you are using almost the same seed. Try moving Random r = new Random(); outside of that method.

Filip Ekberg
  • 36,033
  • 20
  • 126
  • 183