Why is my loop returning the same value from a method that I am calling?

Question

Possible Duplicate:
Random number generator only generating one random number

My code below is doing this and I don't know why:

IF my toEmailAddresses array contains 2 e-mail addresses, uniqueID[0] and uniqueID[1] will return the same value generated from the Util.CreateRandomPassword(16) method call.

If I step through the code, then both uniqueID[0] and uniqueID[1] will contain different values like it should. But if I run the code as usual, for some reason, the same value gets assigned to my uniqueID array: uniqueID[0] and uniqueID[1] will contain the same value.

I even put in the string tempRandomPassword = null and then assign that to the value returned from the CreateRandomPassword method, but that does not work either.

What am I doing wrong?

//toEmailAddresses.Count array will have two e-mail addresses in it.

string[] uniqueID = new string[2];

for (int i = 0; i < toEmailAddresses.Count(); i++)
{
   string tempRandomPassword = null;
   tempRandomPassword = Util.CreateRandomPassword(16);

   uniqueID[i] = tempRandomPassword;
}


        public static string CreateRandomPassword(int passwordLength)
        {
            //http://madskristensen.net/post/Generate-random-password-in-C.aspx

            string allowedChars = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ0123456789";
            char[] chars = new char[passwordLength];
            Random rd = new Random();

            for (int i = 0; i < passwordLength; i++)
            {
                chars[i] = allowedChars[rd.Next(0, allowedChars.Length)];
            }

            return new string(chars);
        }

Required reading: http://csharpindepth.com/Articles/Chapter12/Random.aspx — Oded, Jan 04 '13 at 20:32
`[c#] new Random duplicate` - search SO for this and read the documentation. — , Jan 04 '13 at 20:32
Soner, I would have no idea but if people down vote it has to be for a reason perhaps the answers that they provide are not correct or what the OP is looking for in terms of answering his question — MethodMan, Jan 04 '13 at 20:35
Or someone's just a "funny guy"... doesn't seem to be wrong answers. — jAC, Jan 04 '13 at 20:35
@JanesAbouChleih Although why answer at all with so many duplicates? :-/ (And why is this still open?) — , Jan 04 '13 at 20:36
I don't see any wrong answers at all .. I couldn't tell ya.. — MethodMan, Jan 04 '13 at 20:36
All three answers are exactly right. Someone either doesn't get this stuff, or just has a bad attitude. — Jon B, Jan 04 '13 at 20:36
@pst - this one is actually quite hard to analyze with a debugger. — H H, Jan 04 '13 at 20:37
@pst There were no other answers when I typed it up, but yeah a duplicate for sure. — asawyer, Jan 04 '13 at 20:37
@HenkHolterman Agreed. Even with years of experience this is an easy thing to overlook. — asawyer, Jan 04 '13 at 20:39

score 1 · Answer 1 · edited Jan 04 '13 at 20:37

1

It is because your method uses

 Random rd = new Random();

This local instance of Random will auto-initialize using the clock but when two instances are created too close together (in time) they will use the same seed and produce the same random sequence.

As long as you're not calling this from multiple threads you can use a static field as a simple fix:

    private static Random _rd = new Random();

    public static string CreateRandomPassword(int passwordLength)
    {
        string allowedChars = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ0123456789";
        char[] chars = new char[passwordLength];
        // Random rd = new Random();

        ...
    }

edited Jan 04 '13 at 20:37

Servy

202,030
26
332
449

answered Jan 04 '13 at 20:32

H H

263,252
30
330
514

What do I do if I am calling it from multiple threads? – fraXis Jan 04 '13 at 20:38
You'll need locking. The linked duplicate has a pretty good solution. – H H Jan 04 '13 at 20:40
@fraXis http://csharpindepth.com/Articles/Chapter12/Random.aspx – asawyer Jan 04 '13 at 20:47

score 1 · Answer 2 · answered Jan 04 '13 at 20:32

Your creating a new Random object each time, and since it uses time as a default seed, your getting the same seed, and thus the same numbers.

Make it a field of the class and only create it once.

This is why it worked when you stepped through it by the way. The extra time allowed the seed to change and return different results.

score 1 · Accepted Answer · answered Jan 04 '13 at 20:32

1

You're creating a new instance of Random with the default seed on every invocation. If the seed is the same it will produce the same sequence of numbers. Store the instance and use that for all invocations.

answered Jan 04 '13 at 20:32

Brian Rasmussen

114,645
34
221
317

score -1 · Answer 4 · edited Jan 26 '13 at 20:17

Use thread sleep in the loop

private void button3_Click(object sender, EventArgs e)
        {
            string[] uniqueID = new string[2];
            string[] toEmailAddresses = new string[2];
            toEmailAddresses[0] = "a@1.com";
            toEmailAddresses[1] = "b@1.com";

            for (int i = 0; i < toEmailAddresses.Count(); i++)
            {
                uniqueID[i] = CreateRandomPassword(16);
                System.Threading.Thread.Sleep(10);               
            }

            for (int i = 0; i < uniqueID.Count(); i++)
                MessageBox.Show(i.ToString() + " : " + uniqueID[i]);
        }

`Thread.Sleep` is not a hammer, and `Random` is not a nail. – asawyer Jan 04 '13 at 20:56 — asawyer, Jan 04 '13 at 20:56

Why is my loop returning the same value from a method that I am calling?

4 Answers4