CODEWITHSUNDEEP
excelvbaeditorrows
payal arya
payal arya

Reputation: 11

Deleting rows in excel sheet once a criteria is matched in column

My data looks like this.

enter image description here

I would like to delete rows where the value in column named Position from first has value 1. I wrote a macro to do that which is like this 

Sub DEL_row()
row_number = 2

Do
DoEvents

row_number = row_number + 1
Position_from_first = Sheet8.Range("E" & row_number)

If InStr(Position_from_first, "1") >= 1 Then

Sheet8.Rows(row_number & ":" & row_number).Delete

End If

Loop Until Position_from_first = ""

MsgBox "completed"

End Sub

But when i run it, I get an error. The msg box says completed but it does not do the required job when I go through the excel sheet. Can anyone help me what the problem would be.

Thanks a lot for helping...

Upvotes: 1

Views: 79

Answers (2)

user4039065
user4039065

Reputation:

I gather that you do not want to delete 11, 12, 31, ... etc. so using the InStr function is not an appropriate method of identifying the correct rows to delete. Your sample data image shows column E as having true, right-aligned numbers and the Range.Value2 property can be directly compared against a 1 to determine if the row should be removed.

As mentioned, working from the bottom to the top is important when deleting rows. If you work from the top to the bottom you risk skipping a row when a row has been deleted, everything shifts up and then you increment to the next row.

Option Explicit

Sub DEL_1_rows()
    'declare your variables
    Dim rw As Long

    'this will greatly improve the speed involving row deletion
    Application.ScreenUpdating = False

    With sheet8   '<~~ this is a worksheet CodeName - see below
        For rw = .Cells(Rows.Count, 5).End(xlUp).Row To 2 Step -1
            If .Cells(rw, 5).Value2 = 1 Then .Rows(rw).EntireRow.Delete
        Next rw
    End With

    Application.ScreenUpdating = True

    MsgBox "completed"

End Sub

An AutoFilter Method can speed up identifying the rows to delete. However, a single large bulk deletion can still take a significant amount of processing time.

Option Explicit

Sub DEL_Filtered_rows()

    'this will greatly improve the speed involving row deletion
    Application.ScreenUpdating = False

    With Sheet8   '<~~ this is a worksheet CodeName - see below
        If .AutoFilterMode Then .AutoFilterMode = False
        With .Range(.Cells(1, 5), .Cells(Rows.Count, 5).End(xlUp))
            .AutoFilter field:=1, Criteria1:=1
            With .Resize(.Rows.Count, .Columns.Count).Offset(1, 0)
                If CBool(Application.Subtotal(103, .Cells)) Then
                    .SpecialCells(xlCellTypeVisible).EntireRow.Delete
                End If
            End With
            .AutoFilter
        End With
    End With

    Application.ScreenUpdating = True

    MsgBox "completed"

End Sub

I found it a little odd that you were identifying the worksheet with a Worksheet .CodeName property rather than a Worksheet .Name property. I've included a couple of links to make sure you are using the naming conventions correctly. In any event, I've use a With ... End With statement to avoid repeatedly reidentifying the parent worksheet.

Upvotes: 1

Scott Holtzman
Scott Holtzman

Reputation: 27239

While your concept is spot on, there are some errors in your syntax and your understanding of VBA. I have listed them below.

  1. No need for Do Events in this code at all.
  2. When deleting rows, you need to start from the last row and step back to the beginning in your looping because as you soon as you delete a row it affects all the row references below that, so your loop count will miss rows if you go forward.
  3. You also need to use .EntireRow.Delete. .Delete will only delete cell content.

I have fixed your code below to reflect these issues. I have also provided a simpler alternative to solve your problem below that which uses the AutoFilter method.

Sub DEL_row()
'get last used row in column E
row_number = Sheet8.Range("E" & Sheet8.Rows.Count).End(xlup).Row

For x = row_number to 2 Step -1

     Position_from_first = Sheet8.Range("E" & row_number)

     If InStr(Position_from_first, "1") >= 1 Then
     'If Sheet8.Range("E" & row_number) = 1 ' this will also work

          Sheet8.Rows(row_number & ":" & row_number).EntireRow.Delete

     End If

Next

MsgBox "completed"

End Sub

You can avoid the loop altogether (and save time) if you use AutoFilter, like this:

Sub DEL_row()

With Sheet8

   'get last used row in column E
    row_number = .Range("E" & .Rows.Count).End(xlup).Row

    With .Range("E1:E" & row_number)

         .AutoFilter 1, "1"
         .Offset(1).SpecialCells(xlCellTypeVisible).EntireRow.Delete

    End With

    .AutoFilterMode = False

End With

Msgbox "completed"

End Sub

Upvotes: 2

Related Questions