Slow vba code

[Even]

Hi, this code is very slow, and I don't know why. It is a dropdown list in B1. And whenever I choose a name, it takes long time to calculate. Anybody?

Private Sub Worksheet_Change(ByVal Target As Range)
Dim i As Integer

Application.ScreenUpdating = False
If Not Application.Intersect(Range("B1"), Range(Target.Address)) Is Nothing Then
    Select Case Target.Value
    
    Case Is = "Text":
        For i = 3 To 500
            If Range("AK" & i).Value = "Text" Then
            Rows(i).EntireRow.Hidden = False
            Else: Rows(i).EntireRow.Hidden = True
        End If

    Next i
    
    Case Is = "Text":
        For i = 3 To 500
            If Range("AK" & i).Value = "Text" Then
            Rows(i).EntireRow.Hidden = False
            Else: Rows(i).EntireRow.Hidden = True
        End If
        
        
    
        Application.ScreenUpdating = True

 End Select

    
End Sub

 

[Joe4]

Why do you have the exact same block of code repeated twice?
Do you have any other Event Procedure VBA code running?

Also, note that this:

Range(Target.Address)

can be replaced with just this:

Target

as "Target" is already a range variable.

There is no point in taking a range, getting the address of it, and then making it a range again.
Just use the original range!

 

[JonXL]

I'm surprised this code works at all. Is there possibly something missing from what you posted?

 

[Oaktree]

Instead of looping through the rows and hiding if AK's value is "Text", why not just autofilter column AK for "Text"? Autofilter will be significantly faster than your loop...

 

[Even]

Thank you guys. These are all very good questions to which I don't have answers You see, I had help with the code The "Text" is different in each one. I just wrote text for simplicity. I will tweak around with above suggestions. Regarding the autofilter, I am not familiar to how to do that in a code.

 

[Even]

Joe4: After removing the range, I guess it became a little faster. But it's not Speedy Gonzales

 

[Joe4]

Joe4: After removing the range, I guess it became a little faster. But it's not Speedy Gonzales

That doesn't make it any faster, just a note on some unnecessary stuff that you had in there.

How many different values are you looking for?
Loops are notoriously slow. If you are looping through the entire range fully for each one, that would be pretty slow. There are faster ways of doing it, or at least of limiting the number of loops you have.

But we need to fully understand the scope here:
- How many values are you really looking for?
- Are these values hard-coded (always the same), or are they coming from somewhere else?
- How many rows of data do you really have? One way to limit the number of rows you have to loop through is to dynamically find the last row of data, i.e.

lastRow = Cells(Rows.Count,"AK").End(xlUp).Row
For i = 1 to lastRow
    ...

 

[Rick Rothstein]

I don't see why your code should be exceptionally slow, not for just under 500 rows. However, your code can be compacted as follows (not sure this will be noticeably faster, just more compact)...

Private Sub Worksheet_Change(ByVal Target As Range)
  Dim R As Long
  Application.ScreenUpdating = False
  If Target.Address(0, 0) = "B1" Then
    For R = 3 To 500
      Rows(R).Hidden = (Cells(R, "AK").Value = Range("B1").Value)
    Next
  End If
  Application.ScreenUpdating = True
End Sub

 

[Even]

Rick: That was so much more compact But it didn't perform what I needed. I might need to give more information. Pls stay tuned

 

[Joe4]

Answering the questions I asked in the previous task, and providing some sample data may go a long way in helping us devising a solution that will work for you.