C# - Problems generating a list of randomly generated chars

Question

I am trying to create a list of randomly generated passwords in a WinForms application. In the below code, if I put a breakpoint on the Console.WriteLine(""); line, the specified amount of different passwords are generated with the specified chars. If I do not place a breakpoint here, all of the passwords returned are the same.

I know I am probably missing something very simple, but can't see it. There are no threads, this is happening on the UI thread at this point. I have done some testing with test console and WinForms applications, same result - so I don't think it is a WinForms related issue. Any ideas?

class PasswordGenerator
{
    public List<String> GenerateLowSecurityPasswords(int numOfChars, int numOfPasswords)
    {
        List<String> passwords = new List<String>();

        for (int i = 0; i < numOfPasswords; i++)
        {
            passwords.Add(this.GenerateLowSecurityPassword(numOfChars));
            Console.WriteLine("");
        }

        return passwords;
    }

    private String GenerateLowSecurityPassword(int numOfChars)
    {
        Random rnd = new Random();

        char[] passwd = new char[numOfChars];
        string allowedChars = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ0123456789";

        for (int i = 0; i < numOfChars; i++)
        {
            passwd[i] = allowedChars[rnd.Next(0, allowedChars.Length)];
        }

        return new String(passwd);
    }
}

You need to take `Random rnd = new Random();` at class level. — Hassan, Jun 12 '14 at 06:56
Sjensen85: Make a sleep for one second after each for loop iteration in the top method and check whether the random generator is indeed the same one. — Samuel, Jun 12 '14 at 06:56
@Samuel : Because otherwise the rnd will randomize the same value — Arion, Jun 12 '14 at 06:58
Well see http://msdn.microsoft.com/en-us/library/h343ddh9.aspx it states that the system clock's finite resolution will create the same seed if called close to each other. — Samuel, Jun 12 '14 at 06:59

JensB · Accepted Answer · 2014-06-12T07:17:22.793

I only made a minor change in where the new Random() is placed as when you create a new one in each loop you will get the behavior you are experiencing.

If you are curious as to why you shouldent create a new Random() for every password I would suggest reading this: http://csharpindepth.com/Articles/Chapter12/Random.aspx.

Simplified, it has to do with that your computer can not generate random numbers, it can only generate numbers with a predetermined algorithm (which to us might seem random). So if you create two separate Random() instances with the same seed they will produce the same numbers since they are following the same algorithm

In the edited code below I added DateTime.Now.Millisecond as a seed or starting point for the algorithm. This is just to provide the algorithm with a new starting point each time it is run.

https://dotnetfiddle.net/AFH0Bz

public class Program
{
    public static void Main()
    {
        var pwds = GenerateLowSecurityPasswords(4, 10);
        foreach (var p in pwds)
            Console.WriteLine(p);
    }

    public static List<String> GenerateLowSecurityPasswords(int numOfChars, int numOfPasswords)
    {
        // Moved the creating of the random object here and added a random seed to it so that 
        //it will generate different passwords instead of the same for all
        Random rnd = new Random(DateTime.Now.Millisecond);

        List<String> passwords = new List<String>();
        for (int i = 0; i < numOfPasswords; i++)
        {
              passwords.Add(GenerateLowSecurityPassword(numOfChars, rnd));      
        }

        return passwords;
    }

    private static String GenerateLowSecurityPassword(int numOfChars, Random rnd)
    {

        char[] passwd = new char[numOfChars];
        string allowedChars = "abcdefghijkmnopqrstuvwxyzABCDEFGHJKLMNOPQRSTUVWXYZ0123456789";
        for (int i = 0; i < numOfChars; i++)
        {
            passwd[i] = allowedChars[rnd.Next(0, allowedChars.Length)];
        }

        return new String(passwd);
    }
}

I suggest you add a brief explanation of why your minor change makes the code work. — shree.pat18, Jun 12 '14 at 07:03
These are both right, thanks for the quick responses. Wish I could choose two. Not really sure why I did it this way to begin with. — sjensen85, Jun 12 '14 at 07:17
Just a minor suggestion, but as these are passwords I usually remove characters that look the same such as I, 1, 0, O. Little less confusion and irritation for end users. — JensB, Jun 12 '14 at 07:19

score 1 · Answer 2 · answered Jun 12 '14 at 07:01

Try initializing Random object in the constructor.

class PasswordGenerator
{
    Random rnd;
    public PasswordGenerator()
    {
       rnd = new Random();
    }

    ...
}

The reason is the finite resolution of the clock used to initialize Random. Subsequent initializations of Random will get the same starting position in the random sequence. When reusing the same Random the next value in the random sequence is always generated.

MSDN Remarks:

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.

C# - Problems generating a list of randomly generated chars

2 Answers2