-1

I cannot work out why C# is doing this.

Here's my code;

private string RandomString(int length)
    {
        const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
        string randomString = "";

        for(int i = 0; i < length; i++)
        {
            randomString += chars.ToCharArray()[new Random().Next(chars.ToCharArray().Length)];
        }

        return randomString;
    }

First result: "wwwwwwwwwwwwwwwwwwww"

Second result: "ssssssssssssssssssss"

Third result: "mmmmmmmmmmmmmmmmmmmm"

Lewy Willy
  • 81
  • 1
  • 4
  • 2
    Stop generating a new Random instance every loop iteration. Declare and initialize it once outside the loop. (The loop is going too quickly and you're always getting the same seed.) – Anthony Pegram May 26 '17 at 22:00
  • Make one `Random` object as all your random objects has the same seed (time). – Logman May 26 '17 at 22:02

1 Answers1

1

When you generate a random number generator using new Random(), its seed will be based on the current time, so it will end up being the same thing for each iteration of the loop, since execution will be fast. Instead, you want a var rng = new Random() outside of the loop, and use rng.Next inside the loop.

fuglede
  • 17,388
  • 2
  • 54
  • 99
  • This worked thanks. I thought it would just create a new instance of Random and generate new integer. Weird... – Lewy Willy May 26 '17 at 22:06
  • It certainly does generate a new instance of `Random`, but they all have the same seed, thus generating the same numbers. – fuglede May 27 '17 at 08:06