tags:

views:

48

answers:

2

I have following code that is generating random no. between 1 to 10. This is working fine. But, I do not want doubling in combobox (cmbRnd is my combo box). How can I avoid it?

Private Sub cmdRnd_Click(ByVal sender As Object, ByVal e As EventArgs) Handles cmdRnd.Click
    Dim random As New Random(), i As Integer = 1
    Do While i < 10
        cmbRnd.Items.Add(random.Next(1, 10))
        i = i + 1
    Loop
End Sub
A: 
Dim random As New Random(), i As Integer = 1
Dim cmbRnd As New Windows.Forms.ComboBox
Do While i < 10
    Dim newItem As Int32 = random.Next(1, 10)
    If Not cmbRnd.Items.Contains(newItem) Then
        cmbRnd.Items.Add(newItem)
        i = i + 1
    End If
Loop
Tim Schmelter
While statistically sound, this code is probably going to be quite slow, even for very few numbers.
Konrad Rudolph
@Konrad Rudolph: this code is exactly what the OP asked for. There are definitly faster methods but if you look again at his sourcecode, you will see that performance does not really matter in a button handler for 10 items. So i provided the most straightforward way to check if a Collection contains an Object.
Tim Schmelter
@Tim: Yes, and your answer got an **upvote** from me. My comment was meant as an “oh, by the way,” not as a downvote reason.
Konrad Rudolph
+2  A: 

A better way is to generate an array of numbers 1–10 and then shuffle them.

The easiest way of shuffling numbers is sorting them randomly:

Dim rng As New Random()
Dim numbers As Integer() = Enumerable.Range(1, 10).OrderBy( _
    Function(x) rng.Next()).ToArray()

Next, please read why shuffling numbers like this is a bad idea! and how to do it better.

An unrelated remark:

Don’t use a While loop for iterating over numbers. This special use-case is better mapped by the For loop and people looking at your code will wonder for what reason the For loop was avoided. Consistency is sometimes a bit silly, but in such well-established cases there’s no reason to deviate from the norm.

Konrad Rudolph
Idea looks great but your code produced errorCould you elloborate how to shuffle and then add them to a combo box?Thanks and best regards,Furqan
@user415037: Please try again – there *was* an error in the code, sorry. Also, my code uses Linq which was introduced in the previous release of Visual Studio (VS2008). And the code may require `Imports System.Linq` at the beginning of the file (I forgot to mention that).
Konrad Rudolph
+1 Great answer. One question. Does your algorithm suffer from the same statistical bias as the "naive" algorithm in your link from Jeff? Your algorithm doesn't swap pairs, but assigns a random number to each entry, and then sorts the whole set by the random numbers. So perhaps it doesn't suffer from the same problem of "overswapping" some entries?
MarkJ
@Mark: to be honest, I don’t actually *know* which (if any) statistical weaknesses this method has. But that fact alone should make everyone wary. One *certain* flaw is that it doesn’t assign just *one* random number to each entry – it assigns one different random number each time the sorting algorithm touches a number. First of all, this violates the triangle inequality so *strictly speaking*, the above code is broken at the outset since `OrderBy` requires a relation that satisfies the triangle inequality. Furthermore, it *may* skew the randomization. …
Konrad Rudolph
This linecmbRnd.Items.AddRange(numbers)generated following errorError 1 Value of type '1-dimensional array of Integer' cannot be converted to '1-dimensional array of Object' because 'Integer' is not a reference type.
@user415037: ouch, true. Seems you can’t use `AddRange` after all, in this case.
Konrad Rudolph