0

I have a sheet "Data". With this sheet, I am looking into the column K. If it is red in colour then I am extract the complete row and copy them to another sheet " Delay".

I am following the below code. The code does not have any error but, It just copies 4 rows of red while I have 12 rows.

Could anyone help me to find where I am wrong and what changes I need?

Sub delay()
Dim cell As Range
Dim nextrow As Long
Dim a As Double

Application.ScreenUpdating = False
a = Application.WorksheetFunction.CountA(Sheets("Data").Range("K:K"))
For Each cell In Sheets("Data").Range("K5:K" & a)
If cell.DisplayFormat.Interior.Color = vbRed Then
nextrow = Application.WorksheetFunction.CountA(Sheets("Delayed").Range("K:K"))
Rows(cell.Row).Copy Destination:=Sheets("Delayed").Range("A" & nextrow + 1)
End If

Next
Application.ScreenUpdating = False
End Sub
Jenny
  • 441
  • 2
  • 7
  • 19
  • 2
    What's the rationale for using `CountA`? Is column K guaranteed to be non-empty down to the last used cell in that column? – YowE3K Aug 15 '17 at 09:17
  • @YowE3K no it is not guaranteed – Jenny Aug 15 '17 at 09:36
  • Then what are you trying to achieve with the `CountA`? `CountA` counts how many non-empty cells exist in column K, and you are then using that to generate `"K5:K" & a` and `"A" & nextrow + 1`, i.e. some sort of range. Why does the number or non-empty cells determine the last row you will be processing? – YowE3K Aug 15 '17 at 09:40
  • 2
    Maybe check out [Error in finding last used cell in VBA](https://stackoverflow.com/q/11169445/6535336). – YowE3K Aug 15 '17 at 09:40
  • @YowE3K so you mean to tell instead of using countA I could use lastrow = Range("A1").End(xlDown).Row something like this ? – Jenny Aug 15 '17 at 09:47
  • 1
    No - `Range("A1").End(xlDown).Row` will (assuming A1 and A2 are not empty) find the last non-empty cell before the **first** empty cell. If you were using `CountA` to get the number of non-empty cells and it wasn't returning the total number of cells then you obviously have some empty cells in your range. Therefore you probably want to find the first non-empty cell starting from the bottom of the worksheet. (See the link in my previous comment for ways of doing that.) – YowE3K Aug 15 '17 at 19:21
  • Thank you YowE3k :) – Jenny Aug 16 '17 at 20:08

1 Answers1

2

First of all:
WorksheetFunction.CountA counts the number of cells that are not empty and the values within the list of arguments, you can't use it to count the total number of rows or to find number of the last row (unless all cells are not empty).
What you might need is:

nmbRows = Workbook("WorkbookName").Worksheet("SheetName").Range("K" & Rows.Count).End(xlUp).Row

or something less blunt.
Using CountA can result in constriction of your range of search which will lead in data loss or, in your case, inserting a row in incorrect place.
Example:
Result of

Option Explicit
Sub test()
    Dim a As Long
    a = Application.WorksheetFunction.CountA(ThisWorkbook.Sheets("Ëèñò1").Range("A:A"))
    ThisWorkbook.Sheets("Ëèñò1").Range("B1").Value = a
End Sub

is
enter image description here
Second:
Be cautious, add a reference to each range, sheet as

ThisWorkbook.Sheets("Data").Range("K5:K" & nmbRows)

By doing this you can always be certain you're refering to correct range you want to check.

Another note:
I'm not VBA expert but if I were you I would count number of rows for each sheet respectively at the start of the macro, in loop I would use nextrow=nextrow + 1 construction instead of calling function each time.

AntiDrondert
  • 1,128
  • 8
  • 21
  • 2
    For compatibility with larger sheets, you would be wiser using `nmbRows = Workbook("WorkbookName").Worksheet("SheetName").Range("K" & rows.Count).End(xlUp).Row` instead of row 65,500 up. – CLR Aug 15 '17 at 10:52
  • You should qualify your `Rows` to specify which worksheet you are referring to (especially if there are multiple workbooks open which means some may have a `Rows.Count` of 65536 and others may have 1048576). Therefore use `nmbRows = Workbook("WorkbookName").Worksheet("SheetName").Range("K" & Workbook("WorkbookName").Worksheet("SheetName").Rows.Count).End(xlUp).Row`. – YowE3K Aug 15 '17 at 19:25