C# - Random method returns consecutively repeated values

Question

I'll try to be brief, I think the code is quite simple. I'm trying to create a password generator using a starting string, containing all usable characters (ie "passwordChars") to be used in a FOR loop in association with the Random.Random method to randomly extract a character from the string and then recompose it in a new one to create the password.

private void GeneratePassword()
{
    string passwordChars = null;

    if (isMaiuscEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = "MNBVCXZLKJHGFDSAPOIUYTREWQ";
        }
        else
        {
            passwordChars += "MNBVCXZLKJHGFDSAPOIUYTREWQ";
        }
    }

    if (isLowerEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = "alskdjfhgzmxncbvqpwoeiruty";
        }
        else
        {
            passwordChars += "alskdjfhgzmxncbvqpwoeiruty";
        }
    }

    if (isNumberEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = "2674589103";
        }
        else
        {
            passwordChars += "2674589103";
        }
    }

    if (isSymbolsEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = @"!()-.?[]_`~:;@#$%^&*+=";
        }
        else
        {
            passwordChars += @"!()-.?[]_`~:;@#$%^&*+=";
        }
    }

    int passwordCharsLenght = passwordChars.Length;
    string password = null;

    for (int pwdLenght = 0; pwdLenght < int.Parse(PasswordSizeTextBox.Text); pwdLenght++)
    {
        Random randomizer = new Random();
        int charIndex = randomizer.Next(passwordCharsLenght);

        if (String.IsNullOrEmpty(password) == true)
        {
            password = passwordChars[charIndex].ToString();
        }
        else
        {
            password += passwordChars[charIndex].ToString();
        }
    }

    PasswordOutputTextBlock.Text = password;
}

The problem is that in this way, the result will be a constant repetition of one or more characters, for example:

INPUTS:
Password size = 50, isUpperEnabled = true, isLowerEnabled = true,
isNumberEnabled = true, isSymbolsEnabled = true.

OUTPUT: 
AAAAAAAAAAAAAAAAAAAAAA3333333333333333333333333333

But if I add a MessageBox between the randomizer and the writing of the character obtained in the password strnga then everything works correctly, the password is generated perfectly never generating consecutive repetitions. The code with the inserted MessageBox:

private void GeneratePassword()
{
    string passwordChars = null;

    if (isMaiuscEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = "MNBVCXZLKJHGFDSAPOIUYTREWQ";
        }
        else
        {
            passwordChars += "MNBVCXZLKJHGFDSAPOIUYTREWQ";
        }
    }

    if (isLowerEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = "alskdjfhgzmxncbvqpwoeiruty";
        }
        else
        {
            passwordChars += "alskdjfhgzmxncbvqpwoeiruty";
        }
    }

    if (isNumberEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = "2674589103";
        }
        else
        {
            passwordChars += "2674589103";
        }
    }

    if (isSymbolsEnabled == true)
    {
        if (string.IsNullOrEmpty(passwordChars) == true)
        {
            passwordChars = @"!()-.?[]_`~:;@#$%^&*+=";
        }
        else
        {
            passwordChars += @"!()-.?[]_`~:;@#$%^&*+=";
        }
    }

    int passwordCharsLenght = passwordChars.Length;
    string password = null;

    for (int pwdLenght = 0; pwdLenght < int.Parse(PasswordSizeTextBox.Text); pwdLenght++)
    {
        Random randomizer = new Random();
        int charIndex = randomizer.Next(passwordCharsLenght);

        MessageBox.Show("STOP");

        if (String.IsNullOrEmpty(password) == true)
        {
            password = passwordChars[charIndex].ToString();
        }
        else
        {
            password += passwordChars[charIndex].ToString();
        }
    }

    PasswordOutputTextBlock.Text = password;
}

I hope that I have been clear enough and that someone understands what I am doing wrong. Thanks in advance to everyone.

Does this answer your question? [Random.Next returns always the same values](https://stackoverflow.com/questions/1654887/random-next-returns-always-the-same-values) — Christian Gollhardt, Sep 22 '22 at 21:14
Use `StringBuilder` to build your strings. It makes no sense to have two assignments depending on whether or not anything's been added yet. — Jonathan Wood, Sep 22 '22 at 21:14
Does this answer your question? [Random number generator only generating one random number](https://stackoverflow.com/questions/767999/random-number-generator-only-generating-one-random-number) — n0rd, Sep 22 '22 at 21:31

score 3 · Accepted Answer · answered Sep 22 '22 at 20:50

3

I guess you are using an older .NET version. There the Random instance with the default constructor is using the current time as seed. To avoid repetions you have to move the initialization out of the loop, at the beginning of the method or even as static field:

private void GeneratePassword()
{
    Random randomizer = new Random();
    string passwordChars = null;

    // rest of method remains unchanged ...
}

It's also mentioned in the documentation:

they are instantiated using identical seed values based on the system clock and, therefore, they produce an identical sequence of random numbers

answered Sep 22 '22 at 20:50

Tim Schmelter

450,073
74
686
939

1

@st4ticv0id: you're welcome. Note that the random instance can also be passed as parameter to the method. Then you have more control. Sometimes it's necessary to get the same sequence of random values(unit tests for example or whenever you want that it can be repeated). By initializing it in the method like i did you also are still prone to this issue if you call `GeneratePassword` in a loop (and if you would not have these `MessageBox.Show` calls there). – Tim Schmelter Sep 22 '22 at 21:04
1

@st4ticv0id - Even better is to use this: `private static readonly ThreadLocal __random = new ThreadLocal(() => new Random());`. That is the gold standard is `Random` initialization to avoid this issue. – Enigmativity Sep 22 '22 at 22:04
@Enigmativity: good to know, so if you have an app with many threads and a `static Random` instance you might get into troubles? – Tim Schmelter Sep 22 '22 at 22:09
@TimSchmelter - Yes, `Random` isn't thread-safe. It has internal state that will get right royally stuffed up if used by multiple threads. – Enigmativity Sep 22 '22 at 22:12

C# - Random method returns consecutively repeated values

1 Answers1

Linked