0

I am trying to loop a sub with this code until B9 has a value. And am uncertain what I am doing wrong the sub works until I include the while statement.

Private Sub CommandButton2_Click()

Range("B5:B18").ClearContents

If [D2] = [R15] Then
    Do While IsEmpty("B9") = True
         Randomname
    Loop
End If

Also posting the code for the sub as it is probably relevant. The sub is a random name picker based on code I found and modified to my needs. It compares two ranges and pulls a name from the source range that hasn't already been used, one name at a time.

Sub Randomname()
 Dim source, destination As Range

 Set source = ActiveSheet.Range("L15:L28")
 Set destination = ActiveSheet.Range("B5:B18")
 ReDim randoms(1 To source.Rows.Count)
 destrow = 0
 For i = 1 To destination.Rows.Count
   If destination(i) = "" Then: destrow = i: Exit For
 Next i
 If destrow = 0 Then: MsgBox "no more room in destination range": Exit Sub
 For i = 1 To UBound(randoms): randoms(i) = Rnd(): Next i
 ipick = 0: tries = 0
 Do While ipick = 0 And tries < UBound(randoms)
   tries = tries + 1
   minrnd = WorksheetFunction.Min(randoms)
   For i = 1 To UBound(randoms)
     If randoms(i) = minrnd Then
       picked_before = False
       For j = 1 To destrow - 1
         If source(i) = destination(j) Then: picked_before = True: randoms(i) = 2: Exit For
       Next j
       If Not picked_before Then: ipick = i
       Exit For
     End If
    Next i
 Loop
 If ipick = 0 Then: MsgBox "no more unique name possible to pick": Exit Sub
 destination(destrow) = source(ipick)
End Sub
  • 2
    `IsEmpty("B9")` will always return false. You're probably looking for `IsEmpty([B9])`. – Comintern Feb 09 '17 at 23:59
  • well you got me thinking in the right direction this made it work. Do While IsEmpty(Range("B9").Value) = True thanks. – user3107457 Feb 10 '17 at 00:49
  • @user3107457 Note that `IsEmpty(Range("B9").Value) = True` is the same as `IsEmpty([B9])`. (The `= True` part is redundant because something will only be equal to `True` if it is itself `True`, and `[B9]` is a shortcut for `Range("B9").Value`.) – YowE3K Feb 10 '17 at 02:32

1 Answers1

0

It looks like your syntax is wrong where you have

If destination(i) = "" Then: destrow = i: Exit For

You would be better off with

If desination(i) = "" Then
    destrow = i
    Exit For
End If

When using colons to run multiple commands on the same line, be sure that your syntax is correct. I personally prefer keeping each command on their own line so my code is easier to read. This is particularly important with loops.

Lastly, if you only want to run one command with a then statement you can type it after the then without using a colon. Otherwise, use an if block.

For example

If True = True then msgbox "True"

Or for multiple commands

If True = True Then
    msgbox "True"
    msgbox "Not False"
End if

It is safer, and it makes your code easier to understand.

Brandon Barney
  • 2,382
  • 1
  • 9
  • 18
  • 1
    `If destination(i) = "" Then: destrow = i: Exit For` works just fine. It's ugly as hell, but it compiles and runs. – Comintern Feb 10 '17 at 00:14
  • I'm surprised it compiles successfully and doesn't read it as multiple lines (which would need an End If). Even still, as you noted, its very ugly. It also makes it difficult to read. – Brandon Barney Feb 10 '17 at 01:32
  • See [this question and answers](http://stackoverflow.com/q/41983020/6535336) for some interesting discussion re the use of a colon within single-line `If` statements. – YowE3K Feb 10 '17 at 03:09
  • YowE3K thank you for that resource. That confirms my suspicions that by forcing a single line VBA will run those commands regardless of the result of the condition. – Brandon Barney Feb 10 '17 at 11:24