VBA - How to make code with For Loops and IF Statements more efficient?

Question

I wrote a code to match data (MaterialPN vs. MaterialPS and WeekPN vs. WeekPS) and copy appropriate values between two sheets (Packaging Needs - PN and Packaging Staging - PS).

I already turned off ScreenUpdating, Calculations and Events. This made the run time go from 5 minutes to 1 minute, which is still quite slow (my data is ~3000 rows only). I also tried forcing an exit of the If Statement when the WeekPN is not equal to WeekPS with the use of GoTo Flag1, but this did not make my code run any faster.

Any tips on how to make this code more efficient?

Thanks in advance for any help!

Sub PackagingNeeds2PackagingStaging()
       
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Application.EnableEvents = False

With Sheets("Packaging Needs")
i = .Cells(.Rows.Count, 5).End(xlUp).Row
End With

With Sheets("Packaging Staging")
l = .Cells(.Rows.Count, 1).End(xlUp).Row
End With

j = 25
    For k = 9 To i
        For x = 5 To l
            For Z = 14 To j
            
                MaterialPN = Sheets("Packaging Needs").Cells(k, 5).Value
                MaterialPS = Sheets("Packaging Staging").Cells(x, 1).Value

                WeekPN = Sheets("Packaging Needs").Cells(4, Z).Value
                WeekPS = Sheets("Packaging Staging").Cells(x, 12).Value
                
                If WeekPN <> WeekPS Then GoTo Flag1
                    If WeekPN = WeekPS Then
                        If MaterialPN = MaterialPS Then
                            Sheets("Packaging Staging").Cells(x, 19).Value = Sheets("Packaging Needs").Cells(k, Z).Value
                        End If
                    End If

Flag1:
            Next
        Next
    k = k + 5
    Next
    
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic
Application.EnableEvents = True

End Sub

At least use `Range.Find` to find `Z` without the need for a loop. — Scott Craner, Jun 26 '20 at 20:05
Every time you hit the sheet adds to your execution time, so move the calls which don't use `z` up and into the x or k loops. Also every `.` in a reference costs you, so declare some worksheet variables and use those. That's even without getting into variant arrays. Your `Goto` change doesn't help because it doesn't save any steps - you added an if test that at most only skips the next if test. — Tim Williams, Jun 26 '20 at 20:31
See answer here https://stackoverflow.com/questions/29596432/pointers-needed-for-speeding-up-nested-loop-macro-in-vba/29597193#29597193. `Goto` is very much depreciated and incorrectly used. You are missing an `End If`. — , Jun 26 '20 at 20:38
@TimWilliams Your tips worked perfectly. Code now runs in 5 seconds. Thanks a lot! — Sarah C., Jun 26 '20 at 21:17

score 0 · Accepted Answer · answered Jun 26 '20 at 20:47

A few suggestions - could be faster to use Variant arrays, and also if Match were appropriate here (difficult to say without knowing how many matches you expect to make - if only one then you can also Exit For to break out of a loop once you get a match)

Sub PackagingNeeds2PackagingStaging()
       
    Const J As Long = 25 'use Const for fixed values
    Dim i As Long, x As Long, l As Long, k As Long, z As Long
    Dim shtPN As Worksheet, shtPS As Worksheet
    Dim MaterialPN, MaterialPS, WeekPS
    
    'use worksheet variables
    Set shtPN = ThisWorkbook.Sheets("Packaging Needs")
    Set shtPS = ThisWorkbook.Sheets("Packaging Staging")
    
    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual
    Application.EnableEvents = False
    
    i = shtPN.Cells(shtPN.Rows.Count, 5).End(xlUp).Row
    l = shtPS.Cells(shtPS.Rows.Count, 1).End(xlUp).Row
    
    For k = 9 To i Step 5 '<< use Step instead of your line below
        MaterialPN = shtPN.Cells(k, 5).Value '<< moved this up
        For x = 5 To l
            MaterialPS = shtPS.Cells(x, 1).Value  '<< moved this up
            WeekPS = shtPS.Cells(x, 12).Value     '<< ...and this
            For z = 14 To J
                If shtPN.Cells(4, z).Value = WeekPS Then
                    If MaterialPN = MaterialPS Then
                        shtPS.Cells(x, 19).Value = shtPN.Cells(k, z).Value
                    End If
                End If
            Next z
        Next x
        'k = k + 5 '<<< don't change the counter inside a For loop!
    Next k
        
    Application.ScreenUpdating = True
    Application.Calculation = xlCalculationAutomatic
    Application.EnableEvents = True

End Sub

VBA - How to make code with For Loops and IF Statements more efficient?

1 Answers1