CODEWITHSUNDEEP
vbaexcel
ssoong
ssoong

Reputation: 167

Deletion loop requires multiple runs to delete all rows meeting criteria

I have a msgbox which gets the user to input a value. My workbook then looks up the value in another sheet called 'Values'. In most cases, there are multiple instances of this value in that sheet.

I then take another value from that row (ID) and look for it in a third sheet called 'Req Raw' using the format "[InputValue] [ID]" where ID is a numeric string in the format of "0000".

The workbook then deletes that row in both 'Req Raw' and 'Values' and repeats; continuing to look for the input value in 'Values'.

'SecDelete = Input Value
'VSect = Range in sheet 'Values'
'RReq = Range in sheet 'Req Raw'

With ThisWorkBook.Worksheets("Values")
For Each VSecT In .Range(.Cells(.Cells(Rows.count, 2).End(xlUp).Row, 2), .Cells(15, 2))
    If LCase(VSecT.Value) = LCase(SecDelete) Then
        'Identify ID
        IDF = CStr(VSecT.Offset(columnOffset:=3).Value)
        IDF = Format(IDF, "0000")
        'Find offset ID in 'Req Raw'
        For Each RReq In ThisWorkbook.Worksheets("Req Raw").Range(ThisWorkbook.Worksheets("Req Raw").Cells(ThisWorkbook.Worksheets("Req Raw").Cells(Rows.count, 1).End(xlUp).Row, 1), _
        ThisWorkbook.Worksheets("Req Raw").Cells(2, 1))
            If RReq.Value = VSecT.Value & " " & IDF Then
                'Delete from 'Req Raw'
                RReq.EntireRow.Delete
                Exit For
            End If
        Next RReq
        'Delete from 'Values'
        VSecT.EntireRow.Delete
    End If
Next VSecT
End With

I have found that for some reason, a random number of rows are removed rather than all with the input value.

For example, if my input value was "Test 1.0" and there were ten instances in the 'Values' sheet where "Test 1.0" was present, with corresponding IDs 0000, 0001, 0002, ... 0010, only some of the rows would be deleted each time I run the sub. I have to run the sub 7-8 times before all rows with "Test 1.0" are deleted.

Note that I am looping backwards in both For Each statements.

Upvotes: 1

Views: 657

Answers (4)

Dan Donoghue
Dan Donoghue

Reputation: 6206

I don't like deleting in loops, This code will build a range reference (Rng) and delete all rows in one go at the end.

Sub DeleteBlanks()
Dim Rng As Range, X As Long
For X = 1 To Range("A" & Rows.Count).End(xlUp).Row
    If Range("A" & X).Text = "" Then
        If Rng Is Nothing Then 'This has to be in here because you can't union a range if it is currently nothing
            Set Rng = Range("A" & X)
        Else
            Set Rng = Union(Rng, Range("A" & X))
        End If
    End If
Next
If Not Rng Is Nothing Then Rng.Rows.Delete 'If no blanks are found, this will stop it erroring. Only delete when there is something in Rng
End Sub

Upvotes: 0

Federico Sanchez
Federico Sanchez

Reputation: 145

When I have to delete rows from a set range, what I do while mixing the If statements to search for the rows that will be deleted is, for example, change the value of a the cell in a given column (which i know always has values) to blank. then using the specialcells blanks i select all the new blanks cells and then delete the entire row.

Upvotes: 0

KKowalczyk
KKowalczyk

Reputation: 433

Reason behind this is cell addressing i believe. If you loop through range e.g. a1 to a10 like this

range("a1:a10").select
for each cell in selection
if cell.value = something then
cell.entirerow.delete
end if 
next cell

it is analogical to your code, so what happens when row 2 is deleted? all cells shift upwards so macro on next run skips address A2(which in fact was A3 before deleting) because the loop already passed it. There is the hole in the algorythm, what you need is to go back by one row every time macro deletes a row, so you need to rebuild the macro like this(for my example):

for i = 1 to 10
if range("a" & i ).value = something
  range("a" & i).entirerow.delete
  i = i - 1

next i ' so that each time something is deleted loop steps backward to catch shifted value

hope this helps, cheers

Upvotes: 0

user4039065
user4039065

Reputation:

Here is a portion of your code rewritten to accommodate walking backwards through the rows. Note that I have adjusted your string concatenation as well.

Dim rw1 As Long, rw2 As Long
With ThisWorkbook.Worksheets("Values")
    For rw1 = .Cells(Rows.Count, 2).End(xlUp).Row To 15 Step -1
        If LCase(VSecT.Value) = LCase(SecDelete) Then
            'Identify ID
            IDF = .Cells(rw1, 2).Value & Format(.Cells(rw1, 2).Offset(columnOffset:=3).Value, " 0000")
            'Find offset ID in 'Req Raw'
            With ThisWorkbook.Worksheets("Req Raw")
                For rw2 = .Cells(Rows.Count, 1).End(xlUp).Row To 2 Step -1
                    If .Cells(rw2, 1).Value = IDF Then
                        'Delete from 'Req Raw'
                        .Rows(rw2).EntireRow.Delete
                        Exit For
                    End If
                Next RReq
            End With
            'Delete from 'Values'
            .Rows(rw1).EntireRow.Delete
        End If
    Next rw1
End With

Simply put, you can define a range as .Range("A10:A1") but if you use a For Each/Next to crawl through the cells you will still be progressing through A1, A2, A3.... A10. The numerical row number with Step -1 is the best (only...?) way to work backwards through your data set.

Upvotes: 1

Related Questions