CODEWITHSUNDEEP
vb.netrandom
user287848
user287848

Reputation: 141

Class Random not working as expected

This block is supposed to generate a random 8 character password. When tested, it displays the same 8 characters in the textbox. If I go line by line for the entire iteration, it works properly. If I go line by line for a few loops, then let it run the rest of the characters will be repeat. For example, I break for 3 lines and then resume: abcddddd is my output.

What is causing this to happen?

 Private Sub generateBTN_Click(sender As System.Object, e As System.EventArgs) Handles generateBTN.Click
        generateTXT.Clear()
        While generateTXT.Text.Length < 9
            Dim rn As Random = New Random
            Dim num As Integer = rn.Next(33, 126)
            'disallow ();=
            While num = 40 Or num = 41 Or num = 59 Or num = 61
                num = rn.Next(33, 126)
            End While
            Dim numSTR As String = ChrW(num).ToString
            generateTXT.Text = generateTXT.Text & numSTR
        End While
    End Sub

Upvotes: 0

Views: 58

Answers (3)

dbasnett
dbasnett

Reputation: 11773

Both previous answers have correctly identified the problem with your code as written, the random is being seeded with the same value. A different approach to the problem results in more succinct code and that is easily maintainable.

You have a set of valid characters for the password. They are in the ASCII range of 33 to 126 except for 40, 41, 59, and 61. Why not write that as what it is?

Public Shared ReadOnly validChars As String = "!""#$%&'*+,-./0123456789:<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~"

Remember that a string is an array of characters. Also, what if the users decide that "$&*,. aren't valid characters in the future?

Method1:

Public Shared PRNG As New Random
Public Shared ReadOnly validChars As String = "!""#$%&'*+,-./0123456789:<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~"

Private Sub generateBTN_Click(sender As Object, e As EventArgs) Handles generateBTN.Click
    generateTXT.Text = validChars.Select(Function(c) validChars(PRNG.Next(validChars.Length))).Take(8).ToArray
End Sub

If LINQ is confusing to you try this.

Method2:

Public Shared PRNG As New Random
Public Shared ReadOnly validChars As String = "!""#$%&'*+,-./0123456789:<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~"

Private Sub generateBTN_Click(sender As Object, e As EventArgs) Handles generateBTN.Click
    Dim sb As New System.Text.StringBuilder
    For x As Integer = 1 To 8
        sb.Append(validChars(PRNG.Next(validChars.Length)))
    Next
    generateTXT.Text = sb.ToString
End Sub

Both do the exact same thing. There is no conversion between numbers and characters and they don't waste cycles choosing invalid possibilities. In order to get these to match the OP's code I had to look up numbers to find out which characters were illegal.

Upvotes: 1

Idle_Mind
Idle_Mind

Reputation: 39152

When you create an instance of Random using the default constructor (when you don't pass in anything), it uses the system time as the seed. A specific seed will always generate the same sequence of "random" numbers. When you create a new instance of Random in a tight loop like that, the same seed gets used for each instance because not enough time has transpired to be considered a new seed. Therefore the calls to Next() return the same value since all the instances of Random were seeded with the same time value.

This is why you get different values when you put breakpoints in; enough time has passed between iterations of the loop to be considered a new seed value.

Typical usage of the Random class is to create one instance that gets reused across the whole application. If the Random instance is to be used only within one sub, then declaring it as static keeps it from being disposed across runs (thus preventing the multiple instances using the same time seed problem).

Here's an alternate way to write your code:

Private Sub generateBTN_Click(sender As System.Object, e As System.EventArgs) Handles generateBTN.Click
    Dim num As Integer
    Static rn As New Random
    Dim sb As New System.Text.StringBuilder
    For i As Integer = 1 To 8
        Do
            num = rn.Next(33, 126)
        Loop While num = 40 Or num = 41 Or num = 59 Or num = 61
        sb.Append(ChrW(num))
    Next
    generateTXT.Text = sb.ToString
End Sub

Upvotes: 1

OneFineDay
OneFineDay

Reputation: 9024

You need to declare it outside of the loop and using a Static variable will clean up the problem. Your loop is to fast and the creation was causing to use the same seed as the others. The Static variable fixes that problem nicely. MSDN Random Class Documentation

If the same seed is used for separate Random objects, they will generate the same series of random numbers. This can be useful for creating a test suite that processes random values, or for replaying games that derive their data from random numbers. However, note that Random objects in processes running under different versions of the .NET Framework may return different series of random numbers even if they're instantiated with identical seed values.

Private Sub generateBTN_Click(sender As System.Object, e As System.EventArgs) Handles generateBTN.Click
    ' generateTXT.Clear() 'not needed
    Static rn As New Random
    While generateTXT.Text.Length < 9
        Dim num As Integer = rn.Next(33, 126)
        While num = 40 Or num = 41 Or num = 59 Or num = 61
            num = rn.Next(33, 126)
        End While
        Dim numSTR As String = ChrW(num).ToString
        generateTXT.Text = generateTXT.Text & numSTR
    End While
End Sub

Upvotes: 2

Related Questions