0

The following code is determining if a cell in the row contains the word "Done". If it does then the row is copied to another sheet labled "Log" and removed from the list. Why might the following loop be skipping some rows that are marked "Done"?

Sub LogAndDelete()

Dim r As Integer
Dim Emptyrow As Integer
r = 1
For r = 5 To 70
    If Cells(r, 15) = "Done" Then
        'copying the row marked "Done"
        rng = Range(Cells(r, 1), Cells(r, 7))

        'determining the next empty row on log sheet
        Emptyrow = _
        Application.WorksheetFunction.CountA(Worksheets("Log").Range("A:A")) + 1

        'Pasting onto log sheet
        Range(Worksheets("Log").Cells(Emptyrow, 2), Worksheets("Log").Cells(Emptyrow, 8)) _
        = rng

        'Adding the date removed from log sheet in column A
        Worksheets("Log").Cells(Emptyrow, 1) = Date

        'Removing row marked "done" from list
        Range(Cells(r, 1), Cells(r, 10)).ClearContents
    Else
        r = r + 1
    End If
Next

End Sub

Thank you in advance for the help!

Markus
  • 57
  • 1
  • 10
  • A tip, always use a convention like r c for row column or row, col it makes code a lot more readable by others. – t3dodson May 28 '15 at 20:07
  • Aye Aye Aye, `CountA` to find the next available row? I know you are using `Worksheets("Log").Cells(Emptyrow, 1) = Date` but what if few cells are blank in Col A (got deleted by chance)? To get the last row, use [This](http://stackoverflow.com/questions/11169445/error-in-finding-last-used-cell-in-vba/11169920#11169920) Also how is `rng` declared? – Siddharth Rout May 28 '15 at 20:10
  • I was having the same issue even before I added the logging feature to the loop. So it can't be anything to do the countA – Markus May 28 '15 at 20:14
  • I know :) My above comment was to make it better :) – Siddharth Rout May 28 '15 at 20:14
  • Oh Haha didn't see the link. Thanks! For my case, CountA is working and I think the risk of blanks is not something to be concerned about. But thank you! `With Sheets("Sheet1") LastRow = .Range("E" & .Rows.Count).End(xlUp).Row End With` This works great too! – Markus May 28 '15 at 20:27
  • What should rng be Dimentioned as? – Markus May 28 '15 at 20:39
  • Range :) Also, if you do more complex code in the future, you may want to consider using a For Each loop instead of your current numerical substitution loop with r. – puzzlepiece87 May 28 '15 at 21:14

2 Answers2

6

The problem is in your else case. You are already ignoring that row by going into the else block. and your loop code takes care of incrementing. So your code is basically

If Cells(r, 15) = "Done" Then
    'process done row
Else
    'skip two rows.
End If

I think you should omit your else block and let the loop structure iterate over r.

'remove this part
Else
    r = r + 1
t3dodson
  • 3,949
  • 2
  • 29
  • 40
  • Yup! you're right. Taking out that 2 row skip was right. Don't know why i overlooked that. Thank you! Omit: `Else r = r + 1` – Markus May 28 '15 at 20:30
  • Its pretty easy to skip rows when doing things conditionally I'd be lying if I said I've never done that. – t3dodson May 28 '15 at 21:02
1

I am wondering incrementing i inside a For loop could be the issue. Haven't tested it though.

comment out following line and test.

    i = i + 1
PravyNandas
  • 607
  • 11
  • 12