How to end this loop?

Question

I currently have a VBA Code written to ask for a users input of a string as well as a certain directory, and it searches through each folder, subfolder, workbook and worksheets until it finds the string the user put in. The issue I'm running into is that after it finds the string, it continues to search the rest of the folders. The application I'll be using this in, there is only one of that string being searched. I have tried debugging, and using an if statement with "c" to match str but it keeps throwing an error. The code is attached below, any help is appreciated.

Public WS As Worksheet
Sub SearchWKBooksSubFolders(Optional Folderpath As Variant, Optional Str As Variant)
Dim myfolder As String
Dim a As Single
Dim sht As Worksheet
Dim Lrow As Single
Dim Folders() As String
Dim Folder As Variant
ReDim Folders(0)
If IsMissing(Folderpath) Then
    Set WS = Sheets.Add
    With Application.FileDialog(msoFileDialogFolderPicker)
        .Show
        myfolder = .SelectedItems(1) & "\"
    End With
    Str = Application.InputBox(prompt:="Search string:", Title:="Search all workbooks in a folder", Type:=2)
    If Str = "" Then Exit Sub
    WS.Range("A1") = "Search string:"
    WS.Range("B1") = Str
    WS.Range("A2") = "Path:"
    WS.Range("B2") = myfolder
    WS.Range("A3") = "Folderpath"
    WS.Range("B3") = "Workbook"
    WS.Range("C3") = "Worksheet"
    WS.Range("D3") = "Cell Address"
    WS.Range("E3") = "Link"
    Folderpath = myfolder
    Value = Dir(myfolder, &H1F)
Else
    If Right(Folderpath, 2) = "\\" Then
        Exit Sub
    End If
    Value = Dir(Folderpath, &H1F)
End If
Do Until Value = ""
    If Value = "." Or Value = ".." Then
    Else
        If GetAttr(Folderpath & Value) = 16 Then
            Folders(UBound(Folders)) = Value
            ReDim Preserve Folders(UBound(Folders) + 1)
        ElseIf (Right(Value, 3) = "xls" Or Right(Value, 4) = "xlsx" Or Right(Value, 4) = "xlsm") And Left(Value, 1) <> "~" Then
            On Error Resume Next
            Dim wb As Workbook
            Set wb = Workbooks.Open(Filename:=Folderpath & Value, Password:="zzzzzzzzzzzz")
            On Error GoTo 0
            'If there is an error on Workbooks.Open, then wb Is Nothing:
            If wb Is Nothing Then
                Lrow = WS.Range("A" & Rows.Count).End(xlUp).Row + 1
                WS.Range("A" & Lrow).Value = Value
                WS.Range("B" & Lrow).Value = "Password protected"
            Else
                For Each sht In wb.Worksheets
                    'Expand all groups in sheet
                    sht.Unprotect

                    sht.Outline.ShowLevels RowLevels:=8, ColumnLevels:=8
                    Set c = sht.Cells.Find(Str, After:=sht.Cells(1, 1), LookIn:=xlValues, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext)
                    If Not c Is Nothing Then
                        firstAddress = c.Address
                        Do
                            Lrow = WS.Range("A" & Rows.Count).End(xlUp).Row + 1
                            WS.Range("A" & Lrow).Value = Folderpath
                            WS.Range("B" & Lrow).Value = Value
                            WS.Range("C" & Lrow).Value = sht.Name
                            WS.Range("D" & Lrow).Value = c.Address
                            WS.Hyperlinks.Add Anchor:=WS.Range("E" & Lrow), Address:=Folderpath & Value, SubAddress:= _
                            "'" & sht.Name & "'" & "!" & c.Address, TextToDisplay:="Link"
                            Set c = sht.Cells.FindNext(After:=c)

                        Loop While Not c Is Nothing And c.Address <> firstAddress
                    End If
                Next sht
                wb.Close False
            End If
        End If
    End If
    Value = Dir
Loop
For Each Folder In Folders
    Call SearchWKBooksSubFolders(Folderpath & Folder & "\", Str)
Next Folder
Cells.EntireColumn.AutoFit
End Sub

I rolled back the edit to show the original code. By making code edits, you make answers confusing at best and irrelevant at worst. Your best bet would be to post all your new code in your own answer instead of the tiny snipped you did post. — FreeMan, Jun 30 '17 at 17:12

score 1 · Answer 1 · answered Jun 29 '17 at 18:26

1

Add a boolean variable that you set to True to indicate that you've found what you're looking for. Something like this:

Sub SearchWKBooksSubFolders(Optional Folderpath As Variant, Optional Str As Variant)
  Dim myfolder As String
  Dim a       As Single
  Dim sht     As Worksheet
  Dim Lrow    As Single
  Dim Folders() As String
  Dim Folder  As Variant
  ReDim Folders(0)
  If IsMissing(Folderpath) Then
    Set WS = Sheets.Add
    With Application.FileDialog(msoFileDialogFolderPicker)
      .Show
      myfolder = .SelectedItems(1) & "\"
    End With
    Str = Application.InputBox(prompt:="Search string:", Title:="Search all workbooks in a folder", Type:=2)
    If Str = "" Then Exit Sub
    WS.Range("A1") = "Search string:"
    WS.Range("B1") = Str
    WS.Range("A2") = "Path:"
    WS.Range("B2") = myfolder
    WS.Range("A3") = "Folderpath"
    WS.Range("B3") = "Workbook"
    WS.Range("C3") = "Worksheet"
    WS.Range("D3") = "Cell Address"
    WS.Range("E3") = "Link"
    Folderpath = myfolder
    value = Dir(myfolder, &H1F)
  Else
    If Right(Folderpath, 2) = "\\" Then
      Exit Sub
    End If
    value = Dir(Folderpath, &H1F)
  End If
'---Add this:
  Dim TimeToStop As Boolean
'---Change this:
  Do Until TimeToStop
    If value = "." Or value = ".." Then
    Else
      If GetAttr(Folderpath & value) = 16 Then
        Folders(UBound(Folders)) = value
        ReDim Preserve Folders(UBound(Folders) + 1)
      ElseIf (Right(value, 3) = "xls" Or Right(value, 4) = "xlsx" Or Right(value, 4) = "xlsm") And Left(value, 1) <> "~" Then
        On Error Resume Next
        Dim wb As Workbook
        Set wb = Workbooks.Open(fileName:=Folderpath & value, Password:="zzzzzzzzzzzz")
        On Error GoTo 0
        'If there is an error on Workbooks.Open, then wb Is Nothing:
        If wb Is Nothing Then
          Lrow = WS.Range("A" & Rows.Count).End(xlUp).Row + 1
          WS.Range("A" & Lrow).value = value
          WS.Range("B" & Lrow).value = "Password protected"
        Else
          For Each sht In wb.Worksheets
            'Expand all groups in sheet
            sht.Unprotect

            sht.Outline.ShowLevels RowLevels:=8, ColumnLevels:=8
            Set c = sht.Cells.Find(Str, After:=sht.Cells(1, 1), LookIn:=xlValues, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext)
            If Not c Is Nothing Then
'---Add this
              TimeToStop = True 'since we found what we're looking for
              firstAddress = c.Address
              Do
                Lrow = WS.Range("A" & Rows.Count).End(xlUp).Row + 1
                WS.Range("A" & Lrow).value = Folderpath
                WS.Range("B" & Lrow).value = value
                WS.Range("C" & Lrow).value = sht.Name
                WS.Range("D" & Lrow).value = c.Address
                WS.Hyperlinks.Add Anchor:=WS.Range("E" & Lrow), Address:=Folderpath & value, SubAddress:= _
                                  "'" & sht.Name & "'" & "!" & c.Address, TextToDisplay:="Link"
                Set c = sht.Cells.FindNext(After:=c)
              Loop While Not c Is Nothing And c.Address <> firstAddress
            End If
          Next sht
          wb.Close False
        End If
      End If
    End If
    value = Dir
'---Add these 3 lines
    If Len(value) = 0 Then
      TimeToStop = True
    End If
  Loop
  For Each Folder In Folders
    Call SearchWKBooksSubFolders(Folderpath & Folder & "\", Str)
  Next Folder
  Cells.EntireColumn.AutoFit
End Sub

Do note that you're calling your routine recursively:

  For Each Folder In Folders
    Call SearchWKBooksSubFolders(Folderpath & Folder & "\", Str)
  Next Folder

Once you've gone through all your searching routine, you're going to start all over again because you're calling your Sub from within your Sub. Don't know if this is what you're after, and it may be an additional cause of further unexpected looping.

answered Jun 29 '17 at 18:26

FreeMan

5,660
1
27
53

Thank you! I didn't even realize Excel VBA used Boolean. I will try this out and let you know how it goes. – mmajdalani Jun 29 '17 at 18:33
I am getting an "Object required" Error on line "Lrow = WS.Range("A" & Rows.Count).End(xlUp).Row + 1". I moved timetostop = true to the end of that loop since it still needs to record the datapath and cell location and the link then it needs to stop. But It wont even start recording info now because of the error. – mmajdalani Jun 29 '17 at 18:38
@mmajdalani A) Since `TimeToStop` is _set_ inside the loop, but not _checked_ until the loop finishes processing, _where_ within the loop it is set it won't matter. (It also doesn't matter how many times it's set to `True`, it's only tested once) B) I'm not certain _which_ `Lrow = WS.Range...` row is getting an error, but my bet would be that it's caused by the unqualified call to `Rows.Count`. That will pull from whatever is the _current_ worksheet, and somewhere, somehow, it's probably looking at the wrong one now. – FreeMan Jun 29 '17 at 19:46
Yeah true, The specific Lrow line that is getting an error is right under the "do" And I'm not really sure why it's suddenly throwing the error. I'm fairly new to VBA and I'm still trying to work out kinks in this program. This is really the last thing I need it to do. But I've also tried writing another if statement to check "Value" as well as checking the Value of c but It won't let me do anything with c , it also keeps throwing an object error. I'm not sure what I can do at this point. – mmajdalani Jun 29 '17 at 20:05
Use the debugger and the `Immediate Window` (Ctrl-G). `c` is undeclared, so VBA declares it for you as `variant` the first time you use it. It's an object, so `?c` in the `Immediate Window` will get you another `Object required` error. However, `?c.` (notice the trailing ".") will get IntelliSense to list all the things you _can_ inspect about it so you can figure if it's assigned correctly. – FreeMan Jun 29 '17 at 20:11
Okay, I will try this out when i'm back at work tomorrow. Thanks for your help, please let me know if you can figure out another way to end the loop in the meantime – mmajdalani Jun 29 '17 at 20:12
I would use a `do` loop and with in that loop put an `if` condition statment with a `Exit do`. Super simple, clean and easy to use loop. – Quint Jun 30 '17 at 06:07
@Quint - that is certainly a possibility, by all means, write it up as your own answer. There is nothing inherently wrong with throwing an `Exit` statement in a loop, I've personally never felt they were particularly clean and have generally avoided them. To me, it's much easier to understand the exit condition of the loop if it's built into the loop setup statement (`For x = ...`, `While...`, `Do Until...`, etc.) instead of buried somewhere on a random line within the code the loop is executing. – FreeMan Jun 30 '17 at 11:18
@FreeMan I am trying to do this currently, IntelliSense doesn't seem to be bringing anything up on c. I have a breakpoint right where c is set and I've stepped through the program until c is finally not nothing. I type in ?c. and get nothing – mmajdalani Jun 30 '17 at 11:25
I am looking in the locals window and the type of c is Variant/Object/Range and it lists a bunch of properties but it won't let me call them or see any of them in the code or immediate window – mmajdalani Jun 30 '17 at 11:26
Try inserting `Dim foundCell as Range` above your first `.Find` command, Change all your references to `C` to `foundCell` (a much more readable and descriptive name), and see what you get. I think that your issue may lie in the fact that `c` is automatically defined by VBA as a `Variant` and not as a `Range`, and that's giving you issues. I'd also insert `Option Explicit` at the top of **all** your code modules, then go on to define any variables that the compiler complains about not existing. You'll thank yourself later for doing that - it saves you from your own typos. – FreeMan Jun 30 '17 at 11:51
@mmajdalani "?c at this point returns the searched string" is, I believe, showing the issue you're hitting. You're getting a `String` back when you're expecting a `Range` object to work with. – FreeMan Jun 30 '17 at 11:52
In this case, the "Searched String" is a part number. When it finds this number, ?c is returning that number. I think I was able to figure out how to end the loop, but am now getting an Automation error. I'll update the code. I've never run into an Automation error before and I don't even know what that means if you would have some insight on it – mmajdalani Jun 30 '17 at 11:55
At this point, I think you've drifted off topic from your original question about exiting your loop. Post a new question, with your updated code, asking about the automation error. I'm not sure I've ever run into one of those either. – FreeMan Jun 30 '17 at 12:00
Update: It did not exit the loop/end the program. – mmajdalani Jun 30 '17 at 12:00
The automation error only appeared while i was using the breakpoint debugging method, after just letting it run it didn't exit the loop. – mmajdalani Jun 30 '17 at 12:01
You can use `Exit Do` and `Exit For` to exit `Do` and `For` loops, respectively. Once your condition is met, just add the appropriate `Exit...` statement. – David Zemens Jun 30 '17 at 13:25
@DavidZemens - those are certainly valid and legal options. It's my _personal preference_ to avoid them. To me, it's not as clear and obvious when the loop can end as having some sort of `ExitLoopNow` flag being checked in the loop condition. To me, this is especially important in a long block of code such as what the OP provided. If it were broken up into much shorter blocks, the `Exit [For|Do]` would be much more obvious in 5 lines rather than 50. Again, just my personal preference. – FreeMan Jun 30 '17 at 13:47
I solved it. The recursion was not allowing the End Sub to actually end the program until all folders were searched. I changed "If Str = c.Value Then GoTo 85" to "If Str = c.Value Then End" and it is working now. Thanks to both of you for all your help! – mmajdalani Jun 30 '17 at 16:41

score 0 · Answer 2 · answered Jun 30 '17 at 16:42

0

"If Str = c.Value Then GoTo 85"

Change to

"If Str = c.Value Then End"

answered Jun 30 '17 at 16:42

mmajdalani

37
7

`Exit Sub` is probably more appropriate. Note that `End` has a much larger scope, and while it might work OK for this particular situation, there are other cases where `End` would not be desireable. https://stackoverflow.com/questions/36491908/whats-the-deference-between-end-and-exit-sub-in-vba `End` halts ALL execution. While this sounds tempting to do it also clears all global and static variables (none in this case, but in other situations you may have these). – David Zemens Jun 30 '17 at 16:47
Using a `GoTo` will get you out of your code loop, however it's considered bad practice to do so as it will often lead to nasty, difficult to kill bugs and generally difficult to read and follow spaghetti code. There are occasions where it is an acceptable construct, but generally it's a bad idea. – FreeMan Jun 30 '17 at 17:15

How to end this loop?

2 Answers2