1
\$\begingroup\$
For Each MyIndex In Worksheets
        If MyIndex.Name = "Inv_IB" Or MyIndex.Name = "Inv_MP"             
            With Sheets(MyIndex.Name)
                nFilasActual = .Cells(Rows.Count, 4).End(xlUp).Row
                Set dataRng = .Range("A2", .Range("N" & nFilasActual))
                dataRng.Sort key1:=Range("E2"), order1:=xlAscending
                dataRng.AutoFilter Field:=5, Criteria1:="<>ME"
                Application.DisplayAlerts = False
                dataRng.Offset(1, 0).Resize(dataRng.Rows.Count - 1).SpecialCells(xlCellTypeVisible).Rows.Delete
                Application.DisplayAlerts = True
                .ShowAllData
                nFilasActual = .Cells(Rows.Count, 4).End(xlUp).Row
                Set dataRng = .Range("A2", .Range("N" & nFilasActual))
                dataRng.Sort key1:=Range("C2"), order1:=xlAscending
                dataRng.AutoFilter Field:=3, Criteria1:="<>BODEGA IND"
                Application.DisplayAlerts = False
                dataRng.Offset(1, 0).Resize(dataRng.Rows.Count - 1).SpecialCells(xlCellTypeVisible).Rows.Delete
                Application.DisplayAlerts = True
                .ShowAllData
                nFilasActual = .Cells(Rows.Count, 4).End(xlUp).Row
                dataRng.Sort key1:=Range("H2"), order1:=xlAscending
                .Range("A2:A" & nFilasActual).FormulaR1C1 = "=IF(RC[2]<>R[-1]C[2],1,IF(RC[6]=R[-1]C[6],R[-1]C,R[-1]C+1))"
                .Range("O2").FormulaR1C1 = "=1"
                .Range("O3:O" & nFilasActual).FormulaR1C1 = "=R[-1]C + 1"

            End With
        End If
Next MyIndex

I'm saving info on these 2 sheets from another WorkBook. Then I want to filter this and delete the rest of the info. This is taking too long for what I want. Any ideas for how I can speed this up?

\$\endgroup\$
1
  • \$\begingroup\$ Could you edit the question to include more details about what the code is supposed to do? \$\endgroup\$ Feb 5, 2020 at 22:47

2 Answers 2

3
\$\begingroup\$

The code as is suffers from a few problems, the most obvious at first glance ones being:

Lack of Vertical Spacing

All the code is compressed into very little vertical space. It's not easy to mentally follow along with an infodump. Instead a maintainer would have it easier if they could see logical units in the code.

This can be as easy as adding a single empty line before each section starting with `nFilasActual´.

Lack of Abstraction

This code is indented four levels deep. After the second level of indentation, you should consider extracting the code into a method, just to maintain a cohesive level off semantic abstraction.

Intermingling "high-level" method calls and "low-level" iterations is very much less than ideal. Instead you want code that's in the same procedure to have a roughly equal level of abstraction. This reduces the strain on the reader, because reading requires less context-switching.


Here are a few simplifications I'd make:

Extract Simple Helper Procedures

Since you repeat the following code twice, it's a good candidate for a separate procedure:

Sub SilentDeleteVisibleRows(filteredRange As Range)
    Application.DisplayAlerts = False
    filteredRange.Offset(1, 0).Resize(filteredRange.Rows.Count - 1) _
      .SpecialCells(xlCellTypeVisible).Rows.Delete
    ' This assumes DisplayAlerts was True before calling this method.
    ' Alternatively the state could be stored into a variable.
    Application.DisplayAlerts = True
End Sub

Searching the number of lines is also something you do thrice:

Function NumberOfLines(sheet As Worksheet) As Long
    NumberOfLines = sheet.Cells(Rows.Count, 4).End(xlUp).Row
End Function

You might also want to extract the whole With block into some procedure with a good name to help the readability of the code overall (and reduce the level of indentation, freeing horizontal space).

\$\endgroup\$
1
\$\begingroup\$

Instead of deleting the rows, maybe you should delete (clear) the data you do not want to keep. Then sort the remaining data to move the empty rows at the bottom.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.