For Each Ws Loop not switching sheets

Question

I have a small macro that is supposed to copy/paste data from sheet 1 in Book1 to a fresh workbook (Book2). After that, I want it to loop through the rest of the worksheets from Book1 and copy/paste into Book2 but without the headers.

The macro below completes the first step but then continues to copy/pastes the records in sheet 1 every time instead of switching worksheets to copy/paste new data.

Sub CopyData()

' Copy A:D from all sheets to template

Dim ws As Worksheet
Dim sheetIndex As Integer

sheetIndex = 1

'First Sheet pulls in headers and data

    Windows("Book1.xlsx").Activate
    Sheets(1).Select
    Range("A1:D" & Cells(Rows.Count, "C").End(xlUp).Row).Copy
    Windows("Book2.xlsm").Activate
    ActiveSheet.Paste
    Windows("Book1.xlsx").Activate

'Every other worksheet only copies over data

For Each ws In ActiveWorkbook.Worksheets
   If ws.Index <> 1 Then
    Windows("Book1.xlsx").Activate
    Range("A2:D" & Cells(Rows.Count, "C").End(xlUp).Row).Copy
    Windows("Book2.xlsm").Activate
    Range("A1").End(xlDown).Offset(1).Select
    ActiveSheet.Paste
   End If
    sheetIndex = sheetIndex + 1
Next ws
End Sub

I'm not too experienced so I apologize if the code above isn't optimized. Thanks in advance for your help!

Using `Select` and unqualified `Range()`, `Sheets()` etc. is a recipe for bugs, especially when dealing with multiples workbooks and worksheets. Instead -- use worksheet and workbook variables and properly qualify all ranges. See [How to avoid using Select in Excel VBA](https://stackoverflow.com/q/10714251/4996248) — John Coleman, Jun 22 '18 at 14:11

score 2 · Accepted Answer · answered Jun 22 '18 at 14:12

2

Quick and very dirty solution:

For Each ws In ActiveWorkbook.Worksheets
    ws.Activate
    'rest of the code
Next ws

It would be much better, if you assign the workbook to a variable and loop through the worksheets, without using Activate and Select.

answered Jun 22 '18 at 14:12

Vityata

42,633
8
55
100

Can't believe I forgot to add that in. Works now, thanks! When you say "without using `Activate` and `Select` can you elaborate? Wouldn't I still need to say "variable.Activate" to get back to the proper workbook? Sorry for poor skills :( – Matt Jun 22 '18 at 14:18
@Matt - read the link from the comment of John Coleman. It is quite well explained there. – Vityata Jun 22 '18 at 14:19
See my answer below for an example of what @Vitaya is talking about – Absinthe Jun 22 '18 at 14:20
@Matt this answer is obviously provided merely for completeness' sake. By going down that road you keep your dangerously bug-prone implicit `ActiveSheet` references, and only defer the problems to the next time they bite you in the ...rear end. Don't do this. `Select` and `Activate` are for macro-recorder code, not stuff that you type. – Mathieu Guindon Jun 22 '18 at 14:54
@MathieuGuindon Thanks Mathieu, I am new so have previously gotten my scratch code from the macro-recorder. Glad to know that I can improve on it :) – Matt Jun 22 '18 at 15:14

Absinthe · Answer 2 · 2018-06-22T15:11:19.797

You're almost there but you need to be specific about which sheets and workbooks you're dealing with. Also, you don't need to select them to copy / paste.

Assuming the sheet you're pasting to in Book2.xlsm is Sheet1:

Sub CopyData()

' Copy A:D from all sheets to template

Dim ws As Worksheet, ws2 as worksheet
Dim sheetIndex As Integer
Dim wb1 as workbook, wb2 as workbook

Set wb1 = Workbooks("Book1.xlsx")
set wb2 = Workbooks("Book2.xlsx")

Set ws = wb1.sheets(1)
set ws2 = wb2.sheets(2)


'First Sheet pulls in headers and data

 ws.Range("A1:D" & Cells(Rows.Count, "C").End(xlUp).Row).Copy ws2.range("A1")

'Every other worksheet only copies over data

For Each ws In wb1
   If ws.Index <> 1 Then
       ws.Range("A2:D" & Cells(Rows.Count, "C").End(xlUp).Row).Copy ws2.Range("A1").End(xlDown).Offset(1,0)
   End If
Next ws

End Sub

Thanks for the insight Absinthe. Quick question - I don't see ws1 set in your script but is in the For Each loop, is that supposed to be just 'ws' ? — Matt, Jun 22 '18 at 15:09

score 1 · Answer 3 · answered Jun 22 '18 at 14:34

To achieve something like this, it is important that you know how to initialize workbooks and sheets. Please find time to study how to initialize objects in vba because this will help you in the future.

 Sub CopyData()

' Copy A:D from all sheets to template

Dim ws As Worksheet
Dim sheetIndex As Integer
Dim wbBook1 As Workbook, wbBook2 As Workbook

sheetIndex = 1

'First Sheet pulls in headers and data
Set wbBook1 = ThisWorkbook              'The Workbook where we will copy the data;     This contains the macro
Windows("Book2.xlsx").Activate          'Because we don't know the book name we will just activate it to initialize
                                    'the second workbook where we will copy our data from Book1
Set wbBook2 = ActiveWorkbook

'Every other worksheet only copies over data

'Now that we initialize our two workbooks we will now copy it in the corresponding sheets

For Each ws In wbBook1.Worksheets
    With ws
        If ws.Index = 1 Then
         .Range("A2:D" & Cells(Rows.Count, "C").End(xlUp).Row).Copy wbBook2.Sheets(1).Range("A1")

        Else:
            sheetIndex = sheetIndex + 1
            wbBook2.Worksheets.Add After:=wbBook2.Sheets(sheetIndex - 1) 'Add additional worksheet on the end to paste our other data
            .Range("A2:D" & Cells(Rows.Count, "C").End(xlUp).Row).Copy wbBook2.Sheets(sheetIndex).Range("A1")

        End If
    End With
Next ws
End Sub

I agree. I'll need to actually understand vba better as time goes on, instead of piecing it together through other examples. — Matt, Jun 22 '18 at 14:54

For Each Ws Loop not switching sheets

3 Answers3