Closing Code Causing Major Lag

[Zedinator]

Any idea why these lines of code would be causing my computer to lag and the excel sheet to freeze when closing the sheet?

Private Sub Workbook_BeforeClose(Cancel As Boolean)

Dim f As Long
Dim j As Integer


For f = 1 To Rows.Count
    
    If Cells(f, 1).Interior.ColorIndex = 37 Then
    
        For j = 1 To 13
            Cells(f, j).Interior.Color = xlNone
        Next j
    End If

Next f
        
End Sub

It's supposed to be simple code to remove blue highlighting from the sheet before it closes. Thanks!

 

[Joe4]

Welcome to the Board!

That goes through, every possible row on your page 13 times! You never want to go past your last used row.
Are there any colored cells on the sheet that you want to keep that color (other than blue)?
Is it the whole row (from columns 1 to 13 colored, or just individual cells here and there)?

 

[Zedinator]

Thanks!

There are some rows that are colored red that I want to keep. Entire rows are colored, and I want to be able to take away the blue highlighting and preserve the red. I did not intend for it to loop 13 times, I wanted it to loop through the 13 columns and delete the coloring. I did it this way because when I wrote a code to delete coloring from the entire row at once it also lagged out the sheet.

 

[Joe4]

Try this, I think you will find it to be faster:

Private Sub Workbook_BeforeClose(Cancel As Boolean)

    Dim lr As Long
    Dim r As Long

'   Suppress sheet updating until done with loop
    Application.ScreenUpdating = False

'   Find last used row on sheet
    lr = Range("A1").SpecialCells(xlLastCell).Row
    
'   Loop through each row
    For r = 1 To lr
'       Check to see if column A is highlighted blue
        If Cells(r, 1).Interior.ColorIndex = 37 Then
'           If so, remove highlighting from the first 13 columns of the row
            Range(Cells(r, 1), Cells(r, 13)).Interior.Color = xlNone
        End If
    Next r
    
'   Turn back on sheet updating
    Application.ScreenUpdating = True
    
End Sub

 

[Zedinator]

Much faster, thank you! No lag at all.
Was it the screenupdating feature that fixed things? Why does that improve the speed?

 

[Joe4]

Two big things.

1. Rows.Count counts the total possible number of rows in Excel, which is 1,048,576. So if you are looping through 13 columns, that is over 13+ million loop iterations!
Instead, I told it to find the last row (using the same technique as if you hit CTRL+END while on your sheet). Let's say that was 1,000. Then it would only do 1,000 loop iterations, and if it found blue coloring in column A of a row, it would remove all coloring from the first 13 columns of that particular row. So that would be 1,000 loop iterations instead of 13+ million!

2. Application.ScreenUpdating = False tells Excel to suppress any screen updating. So we run through all our loops, then turn it back on to do the screen updating just once at the end (instead of with every loop).
This is important code to use when you are doing a lot of loops. If you ever see your screen flickering while your code is running, it is because it is updating the screen with every change made in your loop, as opposed to just doing it all at once at the very end.

 

[Zedinator]

Makes sense to me. Thanks for all your help!

 

[Joe4]

You are welcome!

One last piece of advice - you may want to add some "Save" code at the end of the procedure, so it makes sure that all the coloring changes being made are being saved and not lost.
It would just be a line of code, something like:

ActiveWorkbook.Save

 

[Twollaston]

Much faster, thank you! No lag at all.
Was it the screenupdating feature that fixed things? Why does that improve the speed?

It was because you didn’t add the relevant range to the rows.count so normally it would be a range like Activesheet.usedrange.rows.count so it only counts the rows with data