I have to run my code several times for it to execute entirely

Question

I'm not sure whether it's because I'm using a mac or the code is wrong, but the rows aren't identifying properly, and therefore not deleting or pasting it into the other spreadsheet. I have to run the code three times for it to properly go through it and copy/paste and delete the cells into the other spreadsheet.

Many thanks!

here is the code:

Dim j, lastidno As Long


Sheets("Part B + C Modules").Activate
lastidno = Range("A2", Range("A2").End(xlDown)).Count + 1
For j = 2 To lastidno
If Range("O" & j) = "" Then
Sheets("Part B + C Modules").Range("A" & j).Copy
            Sheets("No Options Selected").Select
            NextRow = Cells(Rows.Count, 1).End(xlUp).Row + 1
            Cells(NextRow, 1).Select
            ActiveSheet.Paste
            Sheets("Part B + C Modules").Activate
Rows(j).EntireRow.Delete
End If
Next

MsgBox "done"
End Sub

When deleting rows, always start from the bottom. Here each time two rows that should be deletes are next one each other, the second one is skipped (because after the first one is deleted, the indexes of the next rows are changed) See here: https://stackoverflow.com/questions/57603351/how-to-delete-rows-in-a-loop — Vincent G, Jan 06 '20 at 08:38
See also https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba — Vincent G, Jan 06 '20 at 08:39

JvdV · Accepted Answer · 2020-01-06T10:28:36.200

Iteration and deleting rows goes backwards using a negative Step > For j = lastidno to 2 Step -1

However, it appears you could rewrite your code a bit more elegantly to avoid:

Implicit Range references
Iteration
Use of Activate or Select

The key is to have Explicit sheet references to work with. Also the use of SpecialCells can come in handy here to return a Range in one go (so no more iteration). This way you can also delete all rows in one go!

You code could, for example, look like:

Sub Test()

'Set up your worksheet variables
Dim ws1 As Worksheet: Set ws1 = Worksheets("Part B + C Modules")
Dim ws2 As Worksheet: Set ws2 = Worksheets("No Options Selected")

'Get last used rows
Dim lr1 As Long: lr1 = ws1.Cells(ws1.Rows.Count, 1).End(xlUp).Row
Dim lr2 As Long: lr2 = ws2.Cells(ws2.Rows.Count, 1).End(xlUp).Row

'Set your range and copy it
Dim rng As Range: Set rng = ws1.Range("O2:O" & lr1).SpecialCells(xlCellTypeBlanks).Offset(0, -14)
rng.Copy ws2.Cells(lr2 + 1, 1)

'Delete your range
rng.EntireRow.Delete

MsgBox "done"

End Sub

Small catch: SpecialCells will return an error when no empty cells are found. You might want to work your way around that using On error or count the empty cells in your Range first (my personal preference). So that specific part could looke like:

'Set your range and copy it
If WorksheetFunction.CountBlank(ws1.Range("O2:O" & lr1)) > 0 Then
    Dim rng As Range: Set rng = ws1.Range("O2:O" & lr1).SpecialCells(xlCellTypeBlanks).Offset(0, -14)
    rng.Copy ws2.Cells(lr2 + 1, 1)
End If

Another small note for future reference: Dim j, lastidno As Long only has lastidno declared as Long data type. j Variable is auto-assigned to Variant/Integer so could potentially become a problem when your data is larger than this data type can hold > Return an OverFlow error.

I have to run my code several times for it to execute entirely

1 Answers1