Search code examples
vb.netif-statementrandomcase

Why does VB Random.Next appear misbehave when within an If or Case statement?


I have a snippet of code to select a string randomly from an array. It works fine, except it misbehaves when run within a select-case or If statement.

The following code gives me a random result for Petrol or Diesel, however I only ever get a return of "Electronic Injection Pump" when the randomly selected fuel is "Diesel", and I only get "Carburettor" or "Mechanical Fuel Injection" when the randomly selected fuel is "Petrol".

If I take the fuel system selection code out of the case statement it works as expected giving all the fuel system options. I have tried using an If statement but this gives the same flawed result. The .count values return their expected results of 2 and 3 respectively.

I'm really scratching my head because "Electronic Injection Pump" is second in it's array, while "carburettor" and "Mechanical Fuel Injection" are first and second in their array, so there seems no rhyme or reason to the faulty result.

Dim fuelLr = New Random()
        Dim fuelList() As String = {"Petrol", "Diesel"}
        Dim fuel = fuelList(fuelLr.Next(0, fuelList.Count))

        Dim fuelSystem
        Select Case fuel
            Case "Diesel"
                Dim dieselSystemLr = New Random()
                Dim dieselSystemList() As String = {"Mechanical Injection Pump", "Electronic Injection Pump"}
                fuelSystem = dieselSystemList(dieselSystemLr.Next(0, dieselSystemList.Count))

            Case "Petrol"
                Dim petrolSystemLr = New Random()
                Dim petrolSystemList() As String = {"Carburettor", "Mechanical Fuel Injection", "Electronic Fuel Injection"}
                fuelSystem = petrolSystemList(petrolSystemLr.Next(0, petrolSystemList.Count))

        End Select

Solution

  • The issue is with the constructor of Random using the system clock for its seed. If you instantiate multiple instances of Random in quick succession then you're likely to get the same seed and hence the same set of random values.

    If you write your code like this the problem will go away:

    Dim random = New Random()
    Dim fuelList() As String = {"Petrol", "Diesel"}
    Dim fuel = fuelList(random.Next(0, fuelList.Count))
    Dim fuelSystem = ""
    Select Case fuel
        Case "Diesel"
            Dim dieselSystemList() As String = {"Mechanical Injection Pump", "Electronic Injection Pump"}
            fuelSystem = dieselSystemList(random.Next(0, dieselSystemList.Count))
    
        Case "Petrol"
            Dim petrolSystemList() As String = {"Carburettor", "Mechanical Fuel Injection", "Electronic Fuel Injection"}
            fuelSystem = petrolSystemList(random.Next(0, petrolSystemList.Count))
    End Select
    

    The problem has been resolved in .NET 6 and above. You also then get a special shared instance of random in Random.Shared that is also thread-safe.


    If you want to try to clean up your code a bit and avoid all of the nasty Random calls, here's an extension method that might help:

    Module Extensions
    
        Private __random As New Random() 
        
        <Extension>
        Public Function Sample(OF T)(ByVal source As IEnumerable(Of T)) As T
            Dim collection = TryCast(source, ICollection(Of T))
            Return If( _
                collection Is Nothing, _
                source _
                    .Select(Function (x, n) New With { .Value = x, .Index= n }) _
                    .Aggregate( _
                        CType(Nothing, T), _
                        Function (a, x) If(__random.Next(x.Index + 1) = x.Index, x.Value, a)), _
                collection(__random.Next(collection.Count)))
        End Function
    
    End Module
    

    Now your code can be written with the new Sample operator:

    Dim fuelList() As String = {"Petrol", "Diesel"}
    Dim fuel = fuelList.Sample()
    Dim fuelSystem = ""
    Select Case fuel
        Case "Diesel"
            Dim dieselSystemList() As String = { "Mechanical Injection Pump", "Electronic Injection Pump" }
            fuelSystem = dieselSystemList.Sample()
    
        Case "Petrol"
            Dim petrolSystemList() As String = {"Carburettor", "Mechanical Fuel Injection", "Electronic Fuel Injection"}
            fuelSystem = petrolSystemList.Sample()
    End Select