Inserting rows with VBA too slow

[antonismouf]

Hello all,

I am using a code to identify the value of a cell and then copy its row and insert it above x times depending on the value of the cell. The code is working ok but it is taking a significant amount of time to complete. Does anyone have a suggestion on how I can speed this up?

I found this post: http://www.mrexcel.com/forum/excel-...cations-insert-entire-rows-large-numbers.html but I cant understand what mirabeaus code is doing so I can modify it.

This is the code I am using:

Sub Populate_Adj()

    Dim lastRw As Long
    Dim lastRw1 As Long
    Dim colAmnt As String
    
    lastRw = Cells(Rows.Count, "A").End(xlUp).Row
    
    With Application
        .ScreenUpdating = False
        .Calculation = xlCalculationManual
        .EnableEvents = False
    End With

    
    For nxtRw = lastRw + 1 To 2 Step -1
        
        Select Case Cells(nxtRw, 1).Value
            
            Case "035T"
                If Cells(nxtRw, "A").Font.Bold = False Then
                    For Ins_RW = 1 To 5
                        Cells(nxtRw, "A").EntireRow.Copy
                        Cells(nxtRw, "A").Insert
                    Next
                End If
            Case "135T"
                If Cells(nxtRw, "A").Font.Bold = False Then
                    For Ins_RW = 1 To 10
                        Cells(nxtRw, "A").EntireRow.Copy
                        Cells(nxtRw, "A").Insert
                    Next
                End If
        End Select
    Next

    With Application
        .ScreenUpdating = True
        .Calculation = xlCalculationAutomatic
        .EnableEvents = True
    End With
End Sub

Regards
Antonis

 

[JackDanIce]

Hi Antonis,

Paste all of below into a module and try running procedure Populate_Adj_v1

Sub Populate_Adj_v1()
    
    Dim LR          As Long
    Dim x           As Long
    
    Const s035T     As String = "035T"
    Const s135T     As String = "135T"
           
    With Application
        .ScreenUpdating = False
        .Calculation = xlCalculationManual
        .EnableEvents = False
    End With

    With ActiveSheet
        
        LR = .Range("A" & .rows.Count).End(xlUp).row
        
        For x = LR + 1 To 2 Step -1
            
                If .Range("A" & x).value = s035T And Not .Range("A" & x).Font.Bold Then
                    InsertRows .Range("A" & x), 5
                ElseIf .Range("A" & x).value = s135T And Not .Range("A" & x).Font.Bold Then
                    InsertRows .Range("A" & x), 10
                End If

        Next x
        
    End With
    
    With Application
        .ScreenUpdating = True
        .Calculation = xlCalculationAutomatic
        .EnableEvents = True
    End With
    
End Sub
       
Private Sub InsertRows(ByRef rng As Range, ByRef lRows As Long)

    With rng
        .EntireRow.Copy
        .Resize(lRows).Insert
    End With
    
    Application.CutCopyMode = False
    
End Sub

 

[antonismouf]

Hi Jack,

Thank you very much. The code you provided is way faster. I appreciate the time you took!

Just for educational purposes, why is it so much faster to copy/insert rows with another sub? Is it because you skip the for loops?

 

[JackDanIce]

You're welcome, glad it's an improvement.

I noticed in your code you had a couple of nested loops (loops inside another). It wasn't efficient why you would copy the same range and then a loop to insert 1 row at a time, when you can insert 5 or 10 rows in a single operation, i.e. to insert x numbers of rows is the same as inserting 1 row.

The reason I separated the Insert part out is because if you check your code, the only difference between 035T and 135T is the number of rows you insert so this becomes one of the arguments to the InsertRows procedure (the other being the range being copied). This should all you to add additional checks for say 235T etc.

Other parts, IF.. ELSEIF.. ENDIF I'm informed is quicker to execute than a SELECT CASE statement, though I find SELECT CASE easier to read and navigate.

Using CONST to declare a fixed variable means for each loop, it doesn't need to re-evaluate a string, its already been evaluated and exists in the stack space for the macro.

You don't need to test if a cell.font.bold = TRUE because cell.font.bold can only evaluate to TRUE or FALSE, so putting NOT in front of this, switches TRUE to FALSE and vice-versa and effectively, that line of code changes from:

Yours:

If Range("A" & x).Font.Bold = True Then

To:

If Range("A" & x).Font.Bold Then

 

[antonismouf]

Again, thanks a lot Jack.
I'm new to VBA and programing and your input helps a lot.

Cheers
Antonios

 

[JackDanIce]

Anytime