Search code examples
excelvbafso

Odd behavior when looping through files in a folder


I have macro that loops through the files in a particular folder. I've used this method before without any issues. However, in the current macro, it tries to open the first file (Addresses) again and I'm getting an error message that says "file is already open and being used by another user" (or something to that effect)

There are 2 files in this folder (Addresses / Users). The purpose of this little snippet is to check and make sure each file has data (to prevent issues downstream).

The Address file is good and can be opened and has data. The next file is Users and it is corrupted and cannot be opened, so the On Error GoTo BadFile kicks in. The macro does what I want it to for the bad file but then goes back to the main loop and tries to open the Address file again when the error message is displayed.

There can be 6 different files and I'm trying to determine the status of each file.

X = 1
    
For Each XFILE In xFolder.Files
        
     MyWB = ""
        
     On Error GoTo badfile
        
     objExcelApp.Workbooks.Open (XFILE)
    
     MyWB = ActiveWorkbook.Name
        
     If Len(Trim(Cells(2, 1))) = 0 Then
        
          ReDim Preserve MyFile(1 To X)
          MyFile(X) = ActiveWorkbook.Name
            
          If InStr(1, MyFile(X), "Address") > 0 Then Got1 = True
          If InStr(1, MyFile(X), "Commodities") > 0 Then Got2 = True
          If InStr(1, MyFile(X), "GL") > 0 Then Got3 = True
          If InStr(1, MyFile(X), "Suppliers") > 0 Then Got4 = True
          If InStr(1, MyFile(X), "Users") > 0 Then Got5 = True
          If InStr(1, MyFile(X), "Translate") > 0 Then Got6 = True
            
          objExcelApp.Workbooks(MyWB).Close False
       
BadFile:
            
          ReDim Preserve MyFile(1 To X)
            
          MyWB = XFILE
          MyWB = Replace(MyWB, "\\VS600\", "")
          MyWB = Replace(MyWB, "Testing\", "")
          MyWB = Replace(MyWB, "Data Files\", "")

          MyFile(X) = MyWB
            
          If InStr(1, MyFile(X), "Address") > 0 Then BadFileGot1 = True
          If InStr(1, MyFile(X), "Commodities") > 0 Then BadFileGot2 = True
          If InStr(1, MyFile(X), "GL") > 0 Then BadFileGot3 = True
          If InStr(1, MyFile(X), "Suppliers") > 0 Then BadFileGot4 = True
          If InStr(1, MyFile(X), "Users") > 0 Then BadFileGot5 = True
          If InStr(1, MyFile(X), "Translate") > 0 Then BadFileGot6 = True
           
          X = X + 1
         
     Else
         
          objExcelApp.Workbooks(MyWB).Close False
          
     End If
        
Next XFILE

As I said, it works fine for the Address file and the user file. I would expect it to exit the loop and continue with the rest of the macro but it goes back and looks at the Address file again and I'm not sure why or how to prevent it.

Any help would be greatly appreciated. Thanks for taking the time to look in my issue....


Solution

  • It's hard to tell what exactly fails, but I would assume that using the error handler as you do could cause the problem. You never deactivate the On Error Goto BadFile. That means whenever a runtime error happens in your code, it will jump to that place.

    Now consider any of the statements within the loop, or even after finishing the loop, issues a runtime error: The code will jump back to BadFile:. Whatever the content of XFile is, this file will be processed (again) and handled as "Bad file".

    You shouldn't use Error handling in that way for flow control of your program. In your case, you want to use the Error handler only to check if a workbook could be opened. So limit it's scope to that statement:

        Dim wb As Workbook
        Set wb = Nothing
        On Error Resume Next
        Set wb = Workbooks.Open(file)
        On Error GoTo 0
        
        If wb Is Nothing Then
            ' Handle Error here
        Else
            ' Handle success here
            wb.Close False
        End If
    

    Now, the code will assign a reference to the just opened workbook to the variable wb. If it fails, wb will stay Nothing and you can check for that after disabling the error handler.

    There is much more room for improvement in your code, one thing that hurts most is the Redim Preserve to collect all file names. Instead of using an array, just use a Collection. Collections are build to grow or shrink, while arrays (in VBA) are build for array where you know at a certain point how large it will be. Redim Preserve is a costly action as the whole array needs to be copied.

    Dim MyFiles As New Collection
    For Each XFile in folder.Files
        ...
        MyFiles.Add wb.Name