Excel UDF function runs very slow when selecting entire column

Question

I have a function i got off of MSDN that counts the number of cells in a range that have a another cells color.

Here is the code

Function countCcolor(range_data As Range, criteria As Range) As Long
    Application.Volatile
    Application.ScreenUpdating = False
    Dim datax As Range
    Dim xcolor As Long
xcolor = criteria.Interior.ColorIndex
For Each datax In range_data
    If datax.Interior.ColorIndex = xcolor Then
        countCcolor = countCcolor + 1
    End If
Next datax
Application.ScreenUpdating = True
End Function

The only reason I put the whole column is I need it to update if a value in that column is changed, in reality there will probably never be more than 200 rows (right now there is 85) so it shouldn't be running this slow.

Your problem is with `J:J` you are looping through the entirety of the J column, which is 1 million cells. Slow. — Joe, Aug 05 '14 at 16:46
Yeah I just realized that I have a new problem so I'm editing my question now — grasshopper, Aug 05 '14 at 16:49

score 3 · Accepted Answer · answered Aug 05 '14 at 16:46

3

When I entire into a cell the formula countcColor(j:j, H2) for example it runs very very slow.

Yes, because you're running a volatile function against 1,048,576 rows (Excel 2007+)

If I do J1:4000 it runs fine

I think you answered your own question. Do you really need to check every cell in the column? Most likely not. So you need some way to tell it to "stop". This might help:

For each datax in Application.Intersect(range_data, range_data.Parent.usedRange)

answered Aug 05 '14 at 16:46

David Zemens

53,033
11
81
130

Yeah I just realized that I have a new problem so I'm editing my question now – grasshopper Aug 05 '14 at 16:48
OK, try the suggestion I added to my answer. – David Zemens Aug 05 '14 at 16:50
oh wow this works, you have the best answer but I changed my question so.... round 2? – grasshopper Aug 05 '14 at 16:56
Cheers. If this answered your original Q, please accept it as such. If you now have a different Q, please ask a new one. :) – David Zemens Aug 05 '14 at 16:57
Fyi @user2625177 I'm not aware of any way to make a custom event that responds to changing color format. It might be possible, but I don't think it is built-in to the application- or worksheet-level event procedures. – David Zemens Aug 05 '14 at 16:58
1

OK question changed back to original. – grasshopper Aug 05 '14 at 17:00
For your new Q, see the approach [here](http://www.mrexcel.com/forum/excel-questions/574405-capturing-cell-color-change-visual-basic-applications.html) maybe to get you started? I haven't tried it :) – David Zemens Aug 05 '14 at 17:07

Alter · Answer 2 · 2014-08-05T18:38:29.993

0

In Excel 2010, the max number of rows is 1,048,576. So when you select and Entire column, you're going through all of those rows.

A better way to select a column is:

ActiveSheet.UsedRange.Columns(5)

or (to ignore cells that are formatted but have no value, only works for small files as rows.count + 1 will break if excel file uses max number of rows)

ActiveSheet.Range("A" & ActiveSheet.UsedRange.Rows.Count + 1).End(xlUp)

edited Aug 05 '14 at 18:38

answered Aug 05 '14 at 16:48

Alter

3,332
4
31
56

This isn't useful for the specific case (since a range should be passed, not 'active sheet'). – Joe Aug 05 '14 at 16:48
This was an example of how to get a range from a sheet, you can apply it differently to get more general results. I also find it much more readable than using the standard rng.End(xlUp/xlDown) line – Alter Aug 05 '14 at 16:54
`UsedRange` is generally [*less* reliable than other methods](http://stackoverflow.com/questions/11169445/error-finding-last-used-cell-in-vba), however normally users want to find the last row of "data", which is not always the last row of the `UsedRange`. For this particular use-case, `UsedRange` should work. – David Zemens Aug 05 '14 at 17:10
Good point, but I don't think it's fair to say that it is less reliable. The real issue is that it will accept formatting as an entry. So if a user has a set of cells coloured green/red with no text, it will consider those to be part of the used range. I think it's just [more inclusive](http://stackoverflow.com/questions/7423022/excel-getting-the-actual-usedrange) than xlUp/xlDown. – Alter Aug 05 '14 at 17:56
Took your point into account, made an example with a mix of used range and xlUP – Alter Aug 05 '14 at 18:28
It's less reliable in the sense that *most of the time* when someone wants to find the "last cell" they aren't usually interested in formatted empty cells, but rather the last cell with *data*. I believe it is more inclusive, and that's why it is not reliable for the general "find the last cell with data" approach. But yes, in this case where formatting is desired, it does work where the other methods fail. IN this case, it's the only real way to solve it :) – David Zemens Aug 05 '14 at 18:33
Your revised code actually will fail in the (albeit, unlikely) case that *all* cells have data :) – David Zemens Aug 05 '14 at 18:35
Damn, you're right, you need to check the size. It'll work much faster for small files but will break for large files :/ – Alter Aug 05 '14 at 18:37

score 0 · Answer 3 · answered Aug 05 '14 at 19:23

Perhaps a better solution would be to have some text in the cell with font colour the same as the interior to make it non-visible, eg "R", "Y", "G" etc. Then you can use COUNTIF to count the occurrences of each letter rather than looping to check for interior colour.

Excel UDF function runs very slow when selecting entire column

3 Answers3