0

I am fairly new to VBA and have made the following from combining other users code. The code works and successfully copies rows from one sheet into an index of sheets in another workbook if they contain the target sheet's name. However, it runs very slowly do to looping through multiple activate and select functions. I can't work out how to speed it up and not use activate or select, while still making the code work.

Sub Copy_Rows_Sample()

    Dim wb1 As Workbook, wb2 As Workbook
    Set wb1 = Workbooks.Open("C:\Sample1.xslx")
    Set wb2 = Workbooks.Open("C:\Sample2.xslx")
    
    Dim Page1 As Worksheet
    Set Page1 = wb2.Worksheets("Page1")
    
    wb1.Worksheets(1).Activate
    Excel.Application.DisplayAlerts = False
    Dim wksh As Excel.Worksheet
    Dim jIndex As Integer

    For jIndex = 1 To wb1.Worksheets.Count
    Set wksh = wb1.Worksheets(jIndex)
    With wksh
     
        Dim ran As Range
        For Each ran In Range("A3")
        ran = StrConv(ran.Text, vbProperCase)
        Next
        
        Z = Split(Range("A3").Value, ",")(0)
        ActiveSheet.name = Z
        Dim Y As Variant
        Set Y = ActiveSheet
    
        Page1.Activate
    
        Dim xRow&, NextRow&, LastRow&
        LastRow = Cells.Find(What:="*", After:=Range("A1"), SearchOrder:=xlByRows, SearchDirection:=xlPrevious).Row
    
        For xRow = 1 To LastRow
        NextRow = Y.Cells.Find(What:="*", SearchOrder:=xlByRows, SearchDirection:=xlPrevious).Row + 2
        If WorksheetFunction.CountIf(Rows(xRow), "*" & Z & "*") > 0 Then
        Rows(xRow).Copy Y.Rows(NextRow)
        NextRow = NextRow + 1
        End If
    
        Next xRow
        
        Y.Activate
        wb1.Worksheets(ActiveSheet.Index Mod Worksheets.Count + 1).Select
        
    End With
    Next jIndex
    Excel.Application.DisplayAlerts = True

End Sub
Anders J
  • 1
  • 1
    Have you looked at https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba ? – CLR Aug 22 '22 at 13:03
  • 2
    Couple of things other than the selecting - `For Each ran In Range("A3")` you're telling it to look at each cell within a single cell range. `ran.Text` you're telling it to look at the text as it appears on the sheet and not the value of the cell - may return `###` if your column width is too small. – Darren Bartrup-Cook Aug 22 '22 at 13:29
  • 1
    You use `Dim xRow&` to declare a long, but `Dim jIndex As Integer` to declare an integer - best to be consistent. Memories not that expensive now, so `Dim xRow as Long` is easier to read. – Darren Bartrup-Cook Aug 22 '22 at 13:35
  • 1
    Also have a look at [With...End With](https://learn.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/with-end-with-statement). It's in your code, but you don't make use of it - `For Each ran In Range("A3")` looks at the active sheet, while `For Each ran In .Range("A3")` look at `wkSh` (note the `.` infront of the `Range`. – Darren Bartrup-Cook Aug 22 '22 at 13:43

0 Answers0