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
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