Random object gets non-random with subsequent calls?

Question

Possible Duplicate:
Random number generator only generating one random number

I have a method in my work which generates a sentence by randomly choosing between a set of words as follows:

private string generateTest(string test)
{
    test = test.ToLower();
    string sentence = "";
    for (int c = 0; c < 30; c++)
    {
        Random r = new Random();
        int len = r.Next(3, 7);
        string word = "";
        for (int i = 0; i < len; i++)
        {
            word += test[r.Next(0, test.Length)];
        }
        sentence += word + ' ';
    }
    return sentence;
}

However, the problem is that after the first 6 words are generated randomly, all subsequent generations are exactly the same thing. e.g.

aqaaaz, qqzaq, aqqza, azqq, aazzq, aqqa, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq, azzzq....

Then even when I restart the program, the same thing applies. I am thinking this is the .NET Runtime doing some automatic optimization. Can anyone explain better? How do I get around this problem?

e_ne · Accepted Answer · 2013-01-14T15:16:34.937

5

You're generating your instance for each cycle of the loop. Change your code to:

Random r = new Random();
for (int c = 0; c < 30; c++)
{
...

Also, you might want to assign it a meaningful seed. Right now, you're not passing any to the constructor, which may yield results which are not really random if you recreate the same instance.

EDIT: As Shadow Wizard rightfully pointed out, giving it the same seed yields the same results. When I said "meaningful seed" I implied something that has random or pseudo-random attributes. There are many ways to produce more unpredictable results, but to solve your problem, moving the instance out of the cycle -- Or instantiating it as a member of the containing class -- is enough. Your choice for the seed or for your RNG should depend on your requirements.

edited Jan 14 '13 at 15:16

answered Jan 14 '13 at 15:01

e_ne

8,340
32
43

When you instantiate a random without a seed, the random class uses the current time as a seed. – tom.dietrich Jan 14 '13 at 15:03
Oh, geez, just noticed that. Thanks. But I'm still surprised though, how did it run correctly for the first 6 tries, then subsequently it generated exactly the same thing continuously..? – Chibueze Opata Jan 14 '13 at 15:03
1

-1 for the second part which is wrong. Giving the same seed will result in the same results, and that's NOT what the OP here want. Also, short explanation would improve this answer a lot. – Shadow The GPT Wizard Jan 14 '13 at 15:03
@ChibuezeOpata Tom's comment above answers you perfectly. – e_ne Jan 14 '13 at 15:03
@ShadowWizard Agreed. I've tried to make it a bit clearer. I've left the explanation of why a new random instance will likely generate the same results in a `for` loop to Tom's comment. If the execution is fast enough, the value used for initialization will be the same for each loop. – e_ne Jan 14 '13 at 15:20
OK, better now although I really see not real point in generating a seed here. Using the system time (i.e. the Random class default constructor) is perfectly fine, when of course it's being done once and not in a loop. – Shadow The GPT Wizard Jan 14 '13 at 15:37

Random object gets non-random with subsequent calls?

1 Answers1