0

I have a below code which find the particular value in cell, if that cell is having the value it will delete that row.

Sub FindDelete()  



Set Rng = Range("A:A")

  Set cellFound = Rng.Find("ca-cns")

  Do While Not cellFound Is Nothing

  cellFound.Select

  Selection.EntireRow.Delete

  Set cellFound = Rng.FindNext

Loop



  Set cellFound = Rng.Find("ca-dtc")

  Do While Not cellFound Is Nothing

  cellFound.Select

  Selection.EntireRow.Delete

  Set cellFound = Rng.FindNext

Loop



  Set cellFound = Rng.Find("ca-ext")

  Do While Not cellFound Is Nothing

  cellFound.Select

  Selection.EntireRow.Delete

  Set cellFound = Rng.FindNext

Loop



  Set cellFound = Rng.Find("ca-ns")

  Do While Not cellFound Is Nothing

  cellFound.Select

  Selection.EntireRow.Delete

  Set cellFound = Rng.FindNext

Loop



  Set cellFound = Rng.Find("ca-ssbo")

  Do While Not cellFound Is Nothing

  cellFound.Select

  Selection.EntireRow.Delete

  Set cellFound = Rng.FindNext

Loop



End Sub

It is executed properly, but its not fast and the code is too long. Can this code be minded?

I have ca-cns value 50 times so the whole code is repeated 50 times which makes more time to complete. (It is fast if I filter and delete those row in one go with my hand)

braX
  • 11,506
  • 5
  • 20
  • 33
paran
  • 199
  • 1
  • 3
  • 18
  • 1
    Use a loop and remove the selects. https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba – SJR Apr 23 '20 at 09:47

1 Answers1

2

Try this code please. It determines the last row in A:A, then iterates between the existing values in A:A and creates a new range (rngDel) collecting all cells keeping the necessary to be deleted values. Then, the EntireRow of that range cells are deleted at once:

Sub FindDeleteBis()
 Dim sh As Worksheet, lastRow As Long, rngDel As Range, i As Long
  Set sh = ActiveSheet 'use here your sheet

  lastRow = sh.Range("A" & Rows.count).End(xlUp).Row

  For i = 1 To lastRow
    Select Case sh.Range("A" & i).value
        Case "ca-cns", "ca-dtc", "ca-ext", "ca-ns", "ca-ssbo" 'add here whatever string you need
            If rngDel Is Nothing Then
                Set rngDel = sh.Range("A" & i)
            Else
                Set rngDel = Union(rngDel, sh.Range("A" & i))
            End If
    End Select
  Next
  If Not rngDel Is Nothing Then rngDel.EntireRow.Delete
End Sub

For big ranges try, please the next approach:

Sub FindDeleteBisBis()
Dim sh As Worksheet, arr As Variant, rng As Range, rngDel As Range
Dim lastRow As Long, lastCol As Long, arrHeader As Variant
Set sh = ActiveSheet
lastRow = sh.Range("A" & Rows.count).End(xlUp).Row
lastCol = sh.Cells(1, Columns.count).End(xlToLeft).Column
arrHeader = sh.Range(sh.Range("A1"), sh.Cells(1, lastCol)).value

Set rng = sh.Range(sh.Range("A1"), sh.Cells(lastRow, lastCol))
arr = Array("ca-cns", "ca-dtc", "ca-ext", "ca-ns", "ca-ssbo")

rng.AutoFilter _
    field:=1, _
    Criteria1:=arr, _
    Operator:=xlFilterValues

    Set rngDel = rng.SpecialCells(xlCellTypeVisible)
    rngDel.EntireRow.Delete xlDown
    sh.AutoFilterMode = False

    'recuperate the columns header...
   sh.Rows(1).Insert
   sh.Range("A1").Resize(, lastCol).value = arrHeader
End Sub

On my laptop, it took 193875 milliseconds for 100000 rows...

I took this thread like a challenge... I prepared another solution using arrays and an ingenious way to delete rows. It would be the best if it would not exist the string limitation of 255 characters. I tried to overpass this limitation building strings from the reversed array, up to the limit of 255 chars and delete rows in more steps. The code is faster than the previous one, but not semnificative:

Sub FindDeleteBisBisBis()
 Dim sh As Worksheet, lastRow As Long, arrInit As Variant, arrFin As Variant
 Dim i As Long, arrCond As Variant, k As Long, j As Long, z As Long
  Set sh = ActiveSheet 'use here your sheet

  lastRow = sh.Range("A" & Rows.count).End(xlUp).Row
  arrInit = sh.Range("A1:A" & lastRow).value
  ReDim arrFin(UBound(arrInit) - 1)

  For i = 1 To lastRow
    If isOK(arrInit(i, 1)) Then arrFin(k) = i: k = k + 1
  Next
  If k = 0 Then MsgBox "Sheet already processed...": GoTo final:
   ReDim Preserve arrFin(k - 1)

   Dim strRows As String: ' strRows = "A1"
   For i = k - 1 To 0 Step -1
Restart:
        For j = i To i - 1000 Step -1
            If j < 0 Then Exit For
            If Len(strRows) >= 250 Then Exit For
            z = z + 1
            If strRows = "" Then
                strRows = "A" & arrFin(j)
            Else
                strRows = strRows & ",A" & arrFin(j)
            End If
        Next j
    sh.Range(strRows).EntireRow.Delete
    strRows = "": i = i - z + 1: z = 0: If i < 0 Then Exit For: GoTo Restart
   Next i
final:
End Sub

On my laptop it took 181166 milliseconds for 100000 rows...

Trying to explain to you why the code takes so much time, I had another idea, in order to avoid the discontinuous ranges which kills VBA in terms of time consuming. So, test the next code, please. It will take 2 - 3 seconds...

Sub FindDeleteBisBisBisBis()
 Dim sh As Worksheet, arr As Variant, rng As Range, rngDel As Range
 Dim lastRow As Long, lastCol As Long, arrHeader As Variant
 Set sh = ActiveSheet
 lastRow = sh.Range("A" & Rows.count).End(xlUp).Row
 lastCol = sh.Cells(1, Columns.count).End(xlToLeft).Column
   'Create a new co lumnt to reorder after sorting___________________________
   sh.Cells(1, lastCol + 1).value = "SortOrder"
   sh.Cells(2, lastCol + 1).value = 1: sh.Cells(3, lastCol + 1).value = 2
   sh.Range(sh.Cells(2, lastCol + 1), sh.Cells(3, lastCol + 1)).Select
    sh.Range(sh.Cells(2, lastCol + 1), sh.Cells(3, lastCol + 1)).AutoFill _
        Destination:=sh.Range(sh.Cells(2, lastCol + 1), sh.Cells(lastRow, lastCol + 1))
  '__________________________________________________________________________

 Set rng = sh.Range(sh.Range("A1"), sh.Cells(lastRow, lastCol + 1))
 arr = Array("ca-cns", "ca-dtc", "ca-ext", "ca-ns", "ca-ssbo")

 rng.Sort Key1:=sh.Range("A1"), Order1:=xlAscending, Header:=xlYes

 Dim El As Variant, i As Long, j As Long, firstAddr As String, lastAddr As String
 Dim boolFound As Boolean, iNew As Long
 For Each El In arr
    For i = 2 To lastRow
        If sh.Range("A" & i).value = El Then
            firstAddr = sh.Range("A" & i).Address: iNew = i
            For j = i To lastRow
                If sh.Range("A" & j).value <> sh.Range("A" & j + 1).value Then
                    lastAddr = sh.Range("A" & j).Address: boolFound = True: Exit For
                End If
            Next j
        End If
        If firstAddr <> "" Then
            sh.Range(firstAddr & ":" & lastAddr).EntireRow.Delete
            firstAddr = "": lastAddr = ""
            i = iNew - 1: boolFound = False
        End If
    Next i
 Next
 lastRow = sh.Range("A" & Rows.count).End(xlUp).Row
 Set rng = sh.Range(sh.Range("A1"), sh.Cells(lastRow, lastCol + 1))
 rng.Sort Key1:=sh.Cells(1, lastCol + 1), Order1:=xlAscending, Header:=xlYes
 sh.Range(sh.Cells(1, lastCol + 1), sh.Cells(lastRow, lastCol + 1)).Clear
End Sub
FaneDuru
  • 38,298
  • 4
  • 19
  • 27
  • The code worked very well when there were very few to delete (100), but when I run the code on main file (where 1500+ rows to be deleted from 100K row), it's taking lot time. I waited for 10 mins but it was still in process so I just cancelled the process and deleted manually, which took me normally 7-8 mins. – paran Apr 24 '20 at 20:01
  • @paran: I didn't know that so many rows must be processed... The method is fast enough, but it looks that `rngDel` may freeze Excel if it becomes very big. I tried now some tests and they look like this: For 10000 rows (about 20% occurrences) it takes about 50 seconds. For 20000 rows it keeps the speed (100 sec). For 40000 rows the efficiency decreases a lot. I have in mind a different approach: To split all range in slices of 10000 rows and process each slice separately. – FaneDuru Apr 25 '20 at 08:32
  • @paran: I need putting a marker on the last 1000th row, because after processing, the last row will be different, but I can color the cell interior with something specific and look for it. Now, before I will start that, can you estimate, in percentage, how much the occurrences will mean from that 100000 rows? The efficiency depends on that (the `rngDel` must be kept in a reasonable size)... If not enough efficient, we can try filtering and delete visible cells rows, or using arrays to analize and drop the resulted one instead of the first. But do you have some format which you want to be kept? – FaneDuru Apr 25 '20 at 08:42
  • @paran: I updated my answer with another variant using filtering and deleting the visible cells... – FaneDuru Apr 25 '20 at 09:16
  • @paran: Did you test one of my latest solutions? – FaneDuru Apr 26 '20 at 12:14
  • Not in office, will test tomorrow. – paran Apr 26 '20 at 17:45
  • Tried `FindDeleteBisBis()`, which took 1 min and half for deleting **37834** out of 146694, which I think is slow than human. *Filtering the value took 1 sec*. This means deleting the row is taking the time. – paran Apr 27 '20 at 11:42
  • @paran: I am afraid that your expectations are not realistic enough... For Excel, deleting a row means more then just making it disappearing. It must realocate new row numbers for every cells below, it must do the same regarding the cells formatting, adapt and calculate the formula (if the case), check if there are links on other connections to external sources (even if they do not exist) and so on. VBA must extract `EntireRow` from that discontinue range, also. – FaneDuru Apr 27 '20 at 12:56
  • @paran: If you think that automatically processing 146000 rows in 1 minute and half is too long, you still have the option to do that manually and everybody will be happy... :). But before that, please check the code like I adapted. I mean, I let the code to only select the visible cells. Please, go on Excel sheet and try to **manually** delete visible (selected) cells, read Excel warnings and press OK. Then start your timer and check how much it takes to delete not contiguous ranges. You may have a surprise. And not a pleasant one... :) – FaneDuru Apr 27 '20 at 13:01
  • Apologies for the comment, yes deleting the row is taking time for both (VBA/Human), so your code is the same as required. I thought to test `Sub FindDeleteBisBisBis()`, but I didn't the way your code detects the value which is to be deleted. For eg: ca-cns where it is defined? – paran Apr 27 '20 at 14:18
  • @paran: No, wait some seconds... I have another idea. Try the justify why it works so difficult I found another way. Using sorting and recreate the order adding a new column. I already have the solution (almost) ready. I need to test it... If you still want to clarify the code mechanism, I will explain you whatever you want... :) – FaneDuru Apr 27 '20 at 14:21
  • @paran: Try, please the last version I posted. This proves that contradictory discussions may create progress... :) – FaneDuru Apr 27 '20 at 14:27
  • Now, related to your last comment, `ca-cns` is the first element of `arr` array. The array is defined like that: `arr = Array("ca-cns", "ca-dtc", "ca-ext", "ca-ns", "ca-ssbo")` Then the code iterates between all its elements (`For Each El In arr` ... `Next`). This is only a didactic issue... Now, forget about it and try testing the last version. It is easy to be understood, I think. – FaneDuru Apr 27 '20 at 14:32
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/212627/discussion-between-paran-and-faneduru). – paran Apr 27 '20 at 14:46