Too many If statements

[Yumby]

I want to make sure that the random values I create are unique. So this is the code I use....

Do
    For x = 1 To 3
    thevalue(x) = Int((10 - 1 + 1) * Rnd + 1)
    Next x
    If thevalue(1) <> thevalue(2) Then
        If thevalue(1) <> thevalue(3) Then
            If thevalue(2) <> thevalue(3) Then
            Exit Do
            End If
        End If
    End If
Loop

The problem is that I need to create a lot of values and that means that my code will contain a huge number of If statements. So is there a more efficient way of doing this? Thanks.

[dee-u]

How about storing in a collection those random numbers you are generating and check from there if the new random number is already existing?

[Zvoni]

Sounds like the Lottery-Problem.
Look here in post #3 http://www.vbforums.com/showthread.p...hlight=lottery

[Yumby]

Here's some better code to let you know what's going on. This is the code I need to make more compact.

Do
    For x = 1 To 10
    z = Int((100 - 1 + 1) * Rnd + 1)
    thevalue(x) = thename(z)
    Next x

    For y = 2 To 10
        If thevalue(1) = thevalue(y) Then
        statusvalue = 0
        Exit For
        ElseIf thevalue(1) <> thevalue(y) Then
        statusvalue = 1
            If y = 10 Then
            Exit For
            End If
        End If
    Next y

    For y = 3 To 10
        If statusvalue = 0 Then
        Exit For
        End If
        If thevalue(2) = thevalue(y) Then
        statusvalue = 0
        Exit For
        ElseIf thevalue(2) <> thevalue(y) Then
        statusvalue = 1
            If y = 10 Then
            Exit For
            End If
        End If
    Next y

'And I keep going up until the 9th value is compared with the 10th value.

	For y = 10 To 10
        If statusvalue = 0 Then
        Exit For
        End If
        If thevalue(9) = thevalue(y) Then
        statusvalue = 0
        Exit For
        ElseIf thevalue(9) <> thevalue(y) Then
	statusvalue = 1
            If y = 10 Then
            Exit Do
            End If
        End If
    Next y

Loop

[Zvoni]

And your problem is what exactly?

You need 10 random numbers from 1 to 100 without duplicates, correct?

As i said: the Lottery problem. Using my code from the codebank, you would just have to move the arrDest-Declare outside the function to global (or module-level) scope, and then call it with Call Lottery (10,100)

[Yumby]

I need to make the code more compact (which I have begun to do in the second piece of code I posted) so that I do not have a lot of If statements in my program.

[Zvoni]

Have you even looked at my code? It's some 20 lines long and doesn't even use If-Statements at all

[Yumby]

Yes I have, and I appreciate your help, but for some reason it isn't working for me. At the moment I'm looking at this code...

http://www.vbforums.com/showthread.p...B6-and-earlier

[Zvoni]

And what exactly isn't working? Because i know of some people using my code to their great satisfaction.

[si_the_geek]

Zvoni's code is a much better way to do things, I'd recommend getting it to work if you can... and if you can't do it yourself, let us know the issue(s) so that we can help.

Your code is poor for a variety of reasons, one of which being that you are not actually guaranteed to get a result (it is possible that your code will keep running forever, because you are totally relying on being lucky enough to have 10 different random numbers generated).


As for your code in post #4, there are lots of easy ways to reduce the size.

As an example, take just this part of the code:

    For y = 2 To 10
        If thevalue(1) = thevalue(y) Then
            statusvalue = 0
            Exit For
        ElseIf thevalue(1) <> thevalue(y) Then
            statusvalue = 1
            If y = 10 Then
                Exit For
            End If
        End If
    Next y

(I added proper indenting to make it more readable)

First of all, the "If y = 10 Then / Exit For / End If" part is totally useless. The loop will exit when y=10 anyway, so all that If-block does is make the code larger and slower.

Next up, the "ElseIf" is totally useless too (because you are checking the exact opposite condition of the If, so you already know it must be true - so just an "Else" would do the same thing), and therefore has the same size/speed issues.

On top of that, because the "If" part contains an Exit For, there is no need to use an Else at all.


Taking just those things into account, the chunk of code becomes this:

    For y = 2 To 10
        If thevalue(1) = thevalue(y) Then
            statusvalue = 0
            Exit For
        End If
        statusvalue = 1
    Next y

...and there is no reason to have the statusvalue = 1 line inside the loop (running 9 times in this case), you can move it to before the For (so it runs just once) and it will still have exactly the same effect. In addition to that, it will be easier to read because you know what the default value is before you start reading the loop.


It is also fairly easy to merge all of the For-y loops together to make the code much shorter, but I recommend you don't bother - because even if you do that the code will still be poor compared to Zvoni's method.

[Doogle]

I usually use the Fisher-Yates (Knuth) algorithm to shuffle a set of numbers to ensure no duplicates. No need for any 'If' statements.

Private Function GetRandom(intSmallest As Integer, intLargest As Integer, intNumber As Integer) As Integer()
'
' Returns intNumber of non-repeating positive integer Random Numbers having values between intSmallest and intLargest
'
Dim intTheSet() As Integer
Dim intTempA() As Integer
Dim intI As Integer
Dim intJ As Integer
Dim intRnd As Integer
Dim intTemp As Integer
ReDim intTheSet(intLargest - intSmallest)
ReDim intTempA(intNumber - 1)
'
' Set up the array of numbers required
'
For intI = intSmallest To intLargest
    intTheSet(intJ) = intI
    intJ = intJ + 1
Next intI
'
' Fisher-Yates (Knuth) Shuffle
'
For intI = 0 To UBound(intTheSet)
    intTemp = intTheSet(intI)
    intRnd = Int(Rnd * UBound(intTheSet) + 1)
    intTheSet(intI) = intTheSet(intRnd)
    intTheSet(intRnd) = intTemp
Next intI
'
' Get the first intNumber of numbers
'
For intI = 0 To intNumber - 1
    intTempA(intI) = intTheSet(intI)
Next intI
'
' Assign to the Function's value
'
GetRandom = intTempA()
End Function

Private Sub Command_Click()
Dim intI As Integer
Dim intN() As Integer
'
' Get 10 non-repeating random numbers between 1 and 100
'
intN = GetRandom(1, 100, 10)
'
' Display to the Immediate Window
'
For intI = 0 To UBound(intN)
    Debug.Print intN(intI); " ";
Next intI
Debug.Print
End Sub

Private Sub Form_Load()
Randomize
End Sub

[Yumby]

I've decided to go with merging the For loops together (thanks to Si's tips) as this is the best way forward for me at the moment. Later on I'll check out the other code more closely if I need to. Thanks guys for looking at my code and replying.