0

I'm creating a loop that is supposed to find cells that have dates before 01/01/2019 and if they find they delete the entire row.

The problem is that if they delete per example row number 2, the code is passing to row 3, but since row 2 was deleted previously the original row 3 would be skipped.

I wrote the loop and tried to add "i = 1 - 1" but the excel never stops running the code and blocks the program

Dim ws As Worksheet
Dim Ldate As Date
Dim N As Long

Ldate = "01/01/2019"
N = ws.Cells(Rows.Count, 8).End(xlUp).Offset(0, 0).Row

For i = 1 To N
    If ws.Cells(i, 8).Value < Ldate Then
        ws.Rows(i).Delete
       ' i = 1 - 1          
    End If
Next i
Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
  • Perhaps use a filter and then delete the visible cells. – BigBen Jul 18 '19 at 06:46
  • 2
    Or if you prefer to loop row by row, you need to delete from the bottom up: `For i = N to 1 Step -1`. Also, don't modify `i` inside the loop. – BigBen Jul 18 '19 at 06:55
  • 1
    Possible duplicate of [Excel VBA deleting rows in a for loop misses rows](https://stackoverflow.com/questions/43454139/excel-vba-deleting-rows-in-a-for-loop-misses-rows) – BigBen Jul 18 '19 at 06:57
  • 1
    Don't let Excel guess if this string `"01/01/2019"` is `mm/dd/yyyy` or `dd/mm/yyyy` Never use strings to write a date, instead always use real dates: `Ldate = DateSerial(2019, 1, 1)` to create dates. String-Dates are very evil! The only use case to cast a real date into a string is to print it on paper or screen. All other cases **must** use real dates (number formats) to be reliable. – Pᴇʜ Jul 18 '19 at 06:58
  • By the way, the problem is caused by the fact that `ws.Cells(i, 8).Value < Ldate` is True for empty Rows, too, so after you deleted everything, you stuck in deleting the first empty Row, which is always replaced by the next empty Row. – z32a7ul Jul 18 '19 at 08:08

2 Answers2

2
  1. Don't let Excel guess if this string "01/01/2019" is mm/dd/yyyy or dd/mm/yyyy Never use strings to write a date, instead always use real dates: Ldate = DateSerial(2019, 1, 1) to create dates. String-Dates are very evil! The only use case to cast a real date into a string is to print it on paper or screen. All other cases must use real dates (number formats) to be reliable.

  2. .Offset(0, 0) is usless remove it. Moving 0 rows and 0 columns from current cell will not change current cell at all.

  3. If you delete/add rows then your loops must be backwards in order to row count correctly: For iRow = LastRow To 1 Step - 1

So you end up with something like this

Dim ws As Worksheet
Set ws = ThisWorkbook.Worksheets("Sheet1")

Dim Ldate As Date
Ldate = DateSerial(2019, 1, 1)

Dim LastRow As Long
LastRow = ws.Cells(ws.Rows.Count, 8).End(xlUp).Row

Dim iRow As Long
For iRow = LastRow To 1 Step - 1
    If ws.Cells(iRow , 8).Value < Ldate Then
        ws.Rows(iRow).Delete         
    End If
Next iRow

Alternative if you use a forward loop, you need to collect the rows to delete in a Range variable and delete it at once after the loop is finished. This way deleting doesn't affect the row counting inside the loop (because we delete after the loop):

Dim ws As Worksheet
Set ws = ThisWorkbook.Worksheets("Sheet1")

Dim Ldate As Date
Ldate = DateSerial(2019, 1, 1)

Dim LastRow As Long
LastRow = ws.Cells(ws.Rows.Count, 8).End(xlUp).Row

Dim RowsToDelete As Range    

Dim iRow As Long
For iRow = 1 To LastRow
    If ws.Cells(iRow , 8).Value < Ldate Then
        If RowsToDelete Is Nothing Then
            Set RowsToDelete = ws.Rows(iRow)
        Else
            Set RowsToDelete = Union(RowsToDelete, ws.Rows(iRow))
        End If         
    End If
Next iRow

If Not RowsToDelete Is Nothing Then
    RowsToDelete.Delete
End If

Actually this approach should be faster because it only performs one delete action in the end (instead of one per deleted row).


Edit according comments:

Test if a worksheet exists before you use it.

Option Explicit

Function WorksheetExists(ByVal WorksheetName As String, Optional ByVal InWorkbook As Workbook) As Boolean
    'default workbook is ThisWorkbook
    If InWorkbook Is Nothing Then
        Set InWorkbook = ThisWorkbook
    End If

    Dim ws As Worksheet
    On Error Resume Next
    Set ws = InWorkbook.Worksheets(WorksheetName)
    On Error GoTo 0

    WorksheetExists = Not ws Is Nothing
End Function

Then use it like

If WorksheetExists("Sheet1") Then …

'or to test for a worksheet in a different workbook
If WorksheetExists("Sheet1", Workbooks("OtherWorkbook.xlsx")) Then
Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
  • That it solved my problem, thank you!!! But i would like to ask you something else. If im doing this process to several Worksheets that sometimes exist, other times no, how can i do it? I was reusing the same code, creating a new ws2 and a LastRow2, it works perfect when the sheet exists but gives Error that variable is not set when the sheet doesnt exist – Pedro de Carvalho Jul 18 '19 at 11:36
  • @PedrodeCarvalho then just test if the worksheet exists. See my edit. – Pᴇʜ Jul 18 '19 at 12:05
1

The simplest way is to change

For i = 1 To N

to

For i = N To 1 step -1

This way when your row is deleted it will not change the number of the following rows as these will be lower numbered rows.