user725913
user725913

Reputation:

Random Character Generation Error C#

For some reason, when I try to generate this code all at once, the code produces the same number or letter over and over again (30 times) - see below. When I go through line by line in debug mode, however, the code works great...

private string Generate_ActiveX_name()
{

    StringBuilder charBuilder = new StringBuilder("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ");
    StringBuilder numBuilder = new StringBuilder("0123456789");
    // 
    // Holds Active X Key (final)
    //
    StringBuilder activeX_builder = new StringBuilder();

    // Determine charactar or number


    while (activeX_builder.Length < 30)
    {

        Random activeX_gen = new Random();

        switch (activeX_gen.Next(0, 2))
        {
            case 0:
                Random charSelection = new Random();
                int CharSelected = charSelection.Next(0, 53);
                activeX_builder.Append(charBuilder[CharSelected]);
                break;
            case 1:
                Random numSelection = new Random();
                int NumSelected = numSelection.Next(0, 10);
                activeX_builder.Append(numBuilder[NumSelected]);
                break;

        }
    }

    string activeX_key = activeX_builder.ToString().Substring(0, 8) + "-";
    activeX_key += activeX_builder.ToString().Substring(8, 4) + "-";
    activeX_key += activeX_builder.ToString().Substring(12, 4) + "-";
    activeX_key += activeX_builder.ToString().Substring(16, 11);

    return activeX_key;

}

Why does this code fail when I run it through all at once?

Thanks, Evan

Upvotes: 2

Views: 255

Answers (2)

Alex Aza
Alex Aza

Reputation: 78487

Seems like you are trying to generate GUID.

You can do this with one line

var guid = Guid.NewGuid();

However, here is the fix to your code:

private static string Generate_ActiveX_name()
{
    StringBuilder charBuilder = new StringBuilder("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ");
    StringBuilder numBuilder = new StringBuilder("0123456789");
    StringBuilder activeX_builder = new StringBuilder();


    Random activeX_gen = new Random();
    while (activeX_builder.Length < 30)
    {
        switch (activeX_gen.Next(0, 2))
        {
            case 0:
                int CharSelected = activeX_gen.Next(0, 53);
                activeX_builder.Append(charBuilder[CharSelected]);
                break;
            case 1:
                int NumSelected = activeX_gen.Next(0, 10);
                activeX_builder.Append(numBuilder[NumSelected]);
                break;
        }
    }

    string activeX_key = activeX_builder.ToString().Substring(0, 8) + "-";
    activeX_key += activeX_builder.ToString().Substring(8, 4) + "-";
    activeX_key += activeX_builder.ToString().Substring(12, 4) + "-";
    activeX_key += activeX_builder.ToString().Substring(16, 11);

    return activeX_key;
}

Two problems:

  1. You don't need more than one instance of Random class: charSelection and numSelection are redundant.
  2. Instantiate Random class one time, not inside of the while loop.

[Edit]

8-4-4-11 format:

var guidText = Guid.NewGuid().ToString();
var customGuid = guidText.Substring(0, 18) + guidText.Substring(23, 12);

Upvotes: 1

Reed Copsey
Reed Copsey

Reputation: 564731

You need to move the Random constructor out of your loop. Change this:

while (activeX_builder.Length < 30)
{
    Random activeX_gen = new Random();

To:

Random activeX_gen = new Random();
while (activeX_builder.Length < 30)
{

The issue is that, when an instance of Random is created, it uses the current system clock as a "seed" value for the random number generation. Since your routine runs very quickly, the same seed is chosen every time, so you get the same "random" value. When you're debugging, you're slowing it down (by stepping) enough that a different seed is chosen in each loop, and you get truly random values.

If you move it out of the loop, then a single Random instance will be created, which will lead to a random distribution (and more efficiency!).

Upvotes: 5

Related Questions