1

I am programming an C# Application, which should generate a random Password. My thought was to use the "Random" to generate 10 different Chars and combine them into 1 String.

So I made this Method, which returns a random Char:

public static string RandomChar()
{
    Random random = new Random();
    var chars ="$%#@!*abcdefghijklmnopqrstuvwxyz1234567890?;:ABCDEFGHIJKLMNOPQRSTUVWXYZ^&".ToCharArray();
    string indexValue = chars.GetValue(random.Next(chars.Length)).ToString();           
    return indexValue;
}

This Method works as intended so far.

Then I made the following Method to generate a single string with 10 random chars:

public static string RandomPwd()  
{
    string Pwd = "";
    for(int i = 0; i < 11; i++)
    {
        Pwd= Pwd + RandomChar();
    }
    return Pwd;
}

And then I simply call the RandomPwd() Method from on OnClick event. (I dont think this onClick Event is nessesary to show since the only thing it does there, is calling RandomPwd().)

The weird part is, that if I start the Application with an Breakpoint on 

string a = RandomPwd();

"a" will always be a 10 Characters long string of SAME Charakters, like "UUUUUUUUUU".

But If i set the Breakpoint on the following:

    for(int i = 0; i < 11; i++)
    {
        Pwd= Pwd + RandomChar();
    }
    return Pwd;

Then I can see, that Pwd is getting filled with 10 DIFFERENT Characters. And if I continue stepping through, I also notice, that 

string a = RandomwPwd();

is now 10 Characters long of different Chars.

So it looks like the application works fine, as long as i step through manually, but if I dont, then it wont take different Randoms.

I´ve searched for that Problem on Google and on here, but I couldnt find anything which solves my Problem.

Maybe someone else has experienced that before and would share his Solution to this.

Thanks a lot!

Warsox
  • 63
  • 9
  • 5
    Do NOT create a `Random` inside a method that you call very quickly. Its seed is taken from the system clock, which has a granularity of ~50ms. – Matthew Watson Nov 05 '19 at 12:43
  • Oh wow, I didnt knew that! Thanks for the fast response and the help. – Warsox Nov 05 '19 at 12:46
  • 2
    @MatthewWatson You can get away with it in .NET Core. I don't know the details yet, but when I was creating a snippet for question of this sort to demonstrate the issue in .NET Core I was getting different numbers. – tymtam Nov 05 '19 at 12:47
  • @tymtam Ooh, so they have! That's nice. [Source](https://source.dot.net/#System.Private.CoreLib/shared/System/Random.cs,124) – canton7 Nov 05 '19 at 12:48
  • Here is my gist to generate random string out of some domain. Which is both thread safe and will not be generated out of same seed - https://gist.github.com/eocron/fab4fe6a113fbc1ef15213b9e9e2f8e4. – eocron Nov 05 '19 at 12:52
  • 1
    Good to see that they've fixed this issue in .Net Core - especially given the number of times people make that mistake! – Matthew Watson Nov 05 '19 at 13:12
  • Btw to generate random password use built in solution https://learn.microsoft.com/en-us/dotnet/api/system.web.security.membership.generatepassword – Drag and Drop Nov 05 '19 at 13:13
  • Related: https://stackoverflow.com/questions/54991/generating-random-passwords – Drag and Drop Nov 05 '19 at 13:14

1 Answers1

-2

You need to take Random random = new Random(); from inside the method.

new Random().Next(...) called in a tight loop will often produce the same number. 

for(int i = 0; i< 10; i++)
{
     Random random = new Random();
     Console.WriteLine(random.Next(0,100));
}

produced

17
17
17
17
17
17
17
17
17
17
tymtam
  • 31,798
  • 8
  • 86
  • 126
  • This is completely wrong. – eocron Nov 05 '19 at 12:51
  • @eocron Hey, thanks for feedback, which part is wrong? – tymtam Nov 05 '19 at 12:53
  • 1
    This explains the problem, but is not an answer. – Andrew Nov 05 '19 at 12:57
  • 1
    Your answer is obscure, it does not give insight to why this happening, what could be done and just contains invalid example (though it is valid to pinpoint problem). Also, question is already marked as duplicate and link have a good explanation so it is meaningless to repeat. – eocron Nov 05 '19 at 12:57
  • @eocron Ok, so you think it could be improved, or that is was not necessary. That different to "wrong" :) – tymtam Nov 05 '19 at 13:12