VBA - Create loop for set of active cells with If command

Question

I'm having some difficulty applying the If command I want to execute for all rows of a selected range. So far I've got this:

  Dim x As Integer

  NumRows = Range("J2", Range("J2").End(xlDown)).Rows.Count

  Range("J2").Select

  For x = 1 To NumRows
    If ActiveCell.Value = "9995" Then
    ActiveCell.Offset(0, 7).Value = ActiveCell.Offset(0, 3).Value
    Else: ActiveCell.EntireRow.Delete
    End If
     ActiveCell.Offset(1, 0).Select
  Next

It seems to have no problem selecting a range of cells from J2 down to the end of the list of data. It also has no problem finding cells in this range which contain "9995" and executing the command to copy the data 3 cells to the right off the "9995" cell and paste it 7 cells to the right of the "9995" cell. It even deletes the entire row where the active cell value is not "9995" (which is exactly what I want), but only in some cases. It leaves some rows for which the active cell value is not "9995" and I can't understand why.

Can anyone help?

Thanks!

You have to Keep in mind, that once you delete a row, the next row down becomes the active cell. After the deletion, the command ActiveCell.Offset(1, 0).Select is executed, which means it skips a row, as this row was already selected after the deletion. — Mister 832
– Mister 832, Commented Dec 22, 2016 at 14:04
The corollary of Mister 832's comment is that it's better to loop backwards when deleting. — SJR
– SJR, Commented Dec 22, 2016 at 14:31
Thanks for the input, both. I'll bear this in mind for the future. — Alex
– Alex, Commented Dec 22, 2016 at 17:25

bobajob · Accepted Answer · 2016-12-23 09:44:37Z

1

There's one main issue and a few minor issues here.

The main issue is that whenever you are looping through and deleting rows, you should also loop backwards to avoid issues with the next row not existing any more...

Minor issues include avoiding selections (or ActiveCell) where possible, specifying the worksheet that any Range refers to, using Long rather than Integer, and using Option Explicit (always a good idea).

Putting all that together, try this instead:

Option Explicit

Sub test()
    Dim x As Long
    Dim firstRow As Long
    firstRow = 2
    Dim lastRow As Long
    With ActiveSheet
        lastRow = .Range("J2").End(xlDown).Row
        For x = lastRow To firstRow Step -1
            If .Range("J" & x).Value = "9995" Then
                .Range("J" & x).Offset(0, 7).Value = .Range("J" & x).Offset(0, 3).Value
            Else
                .Range("J" & x).EntireRow.Delete
            End If
        Next
    End With
End Sub

It's also worth pointing out that this code assumes that the data starts on row 2, and has no gaps in column J. If either of these are not true it will not work correctly, so you may want to rethink how you define numRows.

edited Dec 23, 2016 at 9:44

answered Dec 22, 2016 at 14:37

bobajob

1,1926 silver badges13 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Alex Over a year ago

Many thanks bobajob. That's given me a few new concepts to think about. I'm still learning a lot of the basics, so this is a great help.

Alex Over a year ago

The only problem with this seems to be that the command isn't applied to the bottom row when it's finished running. I'm trying to figure out why that might be. I'd guess it's something to do with the line For x = numRows To 2 Step -1.

Alex Over a year ago

Ah, think I've cracked it. I subbed in For x = numRows + 1 To 2 Step -1 and that seems to have done the trick. Thanks again for your help!

bobajob Over a year ago

That would indeed fix it, but I think a better way might be my edited code - by using firstRow and lastRow we can be more explicit about what we are looping through.

Collectives™ on Stack Overflow

VBA - Create loop for set of active cells with If command

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related