1

I want to loop through all open Excel workbooks to identify which one on which to perform operations. Problem is, the code exits the for loop after the active workbook and returns "Nothing" as a result, regardless of how many workbooks I have open.

I need to run this routine weekly to transfer working hours from a downloaded Excel workbook into an alternate workbook. The name of the file changes every week, but always begins with "Timesheets"

I used this routine every week from January through April without any problems. I tried to use it today and this problem cropped up. I've used the routine on several different computers with different operating systems (Windows 7, Windows 10).

I've saved, closed, and reopened the workbook I want to activate to no avail. I don't want to have to change the code every week to access a specific workbook, but use the first 4 letters in the file name to identify the file on which to perform operations.

Sub cmdImportHours_Click()

    Dim ThisWB As Workbook
    Dim ImportWB As Workbook
    Dim WB As Workbook
    Dim msg1 As String
    Dim msg As Variant

' more variables

    msg1 = "Required file not found. Open the import file and try again."

    Set ThisWB = ThisWorkbook
    ThisWB.Worksheets("Hours").Activate

'The following loop exits after one iteration (the active workbook),
'regardless of how many workbooks are open

    For Each WB In Workbooks
        If Left(WB.Name, 4) = "Time" Then
            WB.Activate
            Exit For
        End If
    Next WB

    If WB Is Nothing Then
        msg = MsgBox(msg1, vbOKOnly)
        Exit Sub
    End If

'more code

End Sub

I expect the loop to look at the name of every open Excel workbook, but it exits the For Loop after looking at the active workbook only.

  • As `ThisWorkbook` is always the same (the workbook with your code), it‘s not necessary to give it an extra variable - only the `ActiveWorkbook` changes, if you activate another manually or by code. – Asger May 20 '19 at 06:10
  • As you can reference each `WB` in your code, it's typically not necessary to activate each of them. See here how to [avoid selecting or activating anything](https://stackoverflow.com/q/10714251/10908769). – Asger May 20 '19 at 07:57
  • I'm trying to activate ONE workbook that evaluates to True in the If statement within the For loop. I will only ever have one workbook open that meets the case, but I may have any number of workbooks open at a time. Without specifically naming the workbook (which name changes every week) in the code, I don't know of any other way to activate the correct workbook on which to perform the other operations necessary (that are not part of this discussion and don't need to be displayed in the code window). – Gary Christopher May 21 '19 at 14:53

2 Answers2

1

That because Exit For, just comment that line. And don't use WB outside for each use other variable instead, in this code variable count used to count matching workbooks

Dim count As String
count = 0
For Each WB In Workbooks
    If Left(WB.Name, 4) = "Time" Then
        count = count + 1
        WB.Activate
        'Exit For
    End If
Next WB

If count < 1 Then
    msg = MsgBox(msg1, vbOKOnly)
    Exit Sub
End If
Susilo
  • 816
  • 8
  • 17
  • I'm not trying to count the number of workbooks that evaluate as True. I'm trying to activate the ONLY workbook that is open that evaluates as True. The ```Exit For``` is only activated when the If statement evaluates as true. In the case described, the If statement never evaluates as true, and the loop only runs once before automatically exiting, having never looked at any workbook except the active workbook. – Gary Christopher May 21 '19 at 14:47
1

Your For Each over all workbooks directly returns a usable variable, which references the wanted workbook, so you may even use the variable "ImportWB" here. Exiting the loop by Exit For, if you found the desired item, is good practice.

I introduced two variables for worksheets to use them for copy operations.

Sub cmdImportHours_Click()
    Dim ImportWB As Workbook
    Dim SourceSheet As Worksheet, DestSheet As Worksheet
    Dim msg1 As String
    Dim msg As Variant

    For Each ImportWB In Workbooks
        If Left(ImportWB.Name, 4) = "Time" Then Exit For
    Next ImportWB

    If ImportWB Is Nothing Then
        msg1 = "Required file not found. Open the import file and try again."
        msg = MsgBox(msg1, vbCritical + vbOKOnly, "Workbook not open")
        Exit Sub
    End If

    Set DestSheet = ThisWorkbook.Worksheets("Hours")
    'DestSheet.Activate is typically not necessary
    Set SourceSheet = ImportWB.Sheets(1)

    DestSheet.Range("A2:B10").Value = SourceSheet.Range("A2:B10").Value
    ' more code
End Sub

As ThisWorkbook is always the same (the workbook with your VBA-code), it's not necessary to give it an extra variable, and I omitted it.


If you don't get your already opened workbook by above code, then it's either opened within a protected view ...

For i = 1 To Application.ProtectedViewWindows.Count
    Debug.Print "Protected Workbook: " & _
        Application.ProtectedViewWindows(i).Workbook.Name
Next i

... or within another Excel instance. In that case you may reference it by e. g.

Set ImportWB = GetObject("Path\Filename.xlsx")

See here for more examples on that.

Asger
  • 3,822
  • 3
  • 12
  • 37