Is this countifs VBA code OK, or will it crash?

Question

I've been told my countifs code is bad practice and worried it might crash. I have over 200 rows of code like this. Do I need to add variables or loops to make it better? Other lines have different criteria and more criteria, so it's pretty involved. My data only has up to 100,000 rows and it works OK...only takes a few seconds to complete. I have heaps of other code doing other stuff but this is the only part I was told is bad practice. I'm a newbie with no variable experience. Any suggestions much appreciated, thanks Cobber.

'Counts the data on sheet 2 and populates on sheet 1
'counts total, pass and fail
Sheets(1).Range("D5").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "RDG1", Range("G7:G10000"), "Sewer Works")
'counts pass
Sheets(1).Range("E5").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "RDG1", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "PASS")
'counts fail
Sheets(1).Range("F5").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "RDG1", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "FAIL")
'seperate areas
Sheets(1).Range("D6").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "TYH2", Range("G7:G10000"), "Sewer Works")
Sheets(1).Range("E6").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "TYH2", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "PASS")
Sheets(1).Range("F6").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "TYH2", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "FAIL")

Sheets(1).Range("D7").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "UJM3", Range("G7:G10000"), "Sewer Works")
Sheets(1).Range("E7").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "UJM3", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "PASS")
Sheets(1).Range("F7").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "UJM3", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "FAIL")

Sheets(1).Range("D8").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "OPL4", Range("G7:G10000"), "Sewer Works")
Sheets(1).Range("E8").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "OPL4", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "PASS")
Sheets(1).Range("F8").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "OPL4", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "FAIL")

Sheets(1).Range("D9").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "HUJ5", Range("G7:G10000"), "Sewer Works")
Sheets(1).Range("E9").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "HUJ5", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "PASS")
Sheets(1).Range("F9").Value = Application.WorksheetFunction.CountIfs(Range("C7:C10000"), "HUJ5", Range("G7:G10000"), "Sewer Works", Range("I7:I10000"), "FAIL")

'calculates percent score after the columns are populated on sheet 1
Range("G5").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G6").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G7").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G8").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G9").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"

Range("G10").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G11").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G12").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G13").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"
Range("G14").Select
ActiveCell.FormulaR1C1 = "=RC[-2]/RC[-3]"

There's a lot of repetition, and your ranges should be scoped to a specific sheet object, otherwise your code will not be robust. Try factoring out the CountIfs into a separate function. — Tim Williams, Oct 30 '19 at 06:27
The last part reduces to `Range("G5:G14").FormulaR1C1 = "=RC[-2]/RC[-3]"` — Tim Williams, Oct 30 '19 at 06:28
You might benefit from reading [How to avoid using Select in Excel VBA](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba). — Pᴇʜ, Oct 30 '19 at 10:59

Tim Williams · Answer 1 · 2019-10-30T06:44:03.407

0

As commented above - a lot of repetition and ranges not scoped to specific worksheets.

Here's a partial clean-up for the first section of counts: your countifs are factored out into a separate function called from your main code.

With ThisWorkbook.Sheets(1)
    .Range("D5").Value = Counts("RDG1", "Sewer Works")
    .Range("E5").Value = Counts("RDG1", "Sewer Works", "PASS")
    .Range("F5").Value = Counts("RDG1", "Sewer Works", "FAIL")

    'other counts here

End With



'Get row counts from sheet2
'  Optional 3rd parameter for PASS/FAIL
Function Counts(v1 As String, v2 As String, Optional v3 As String = "") As Long
    Dim rng1 As Range, rng2 As Range, rng3 As Range
    With ThisWorkbook.Sheets(2)
        Set rng1 = .Range("C7:C10000")
        Set rng2 = .Range("G7:G10000")
        Set rng3 = .Range("I7:I10000")
        If Len(v3) > 0 Then
            Counts = Application.CountIfs(rng1, v1, rng2, v2, rng3, v3)
        Else
            Counts = Application.CountIfs(rng3, v1, rng2, v2)
        End If
    End With
End Function

edited Oct 30 '19 at 06:44

answered Oct 30 '19 at 06:38

Tim Williams

154,628
8
97
125

Wow, so quick. Just to clarify, I still need all the rows I used where you say 'other counts here, but your method makes it better? I'll give it a go, thanks. – Cobber Oct 30 '19 at 06:45
Yes - you need to add your other counts - I just gave the first 3 as an example – Tim Williams Oct 30 '19 at 06:47
Hi, I tried your code but it bugs out at With ThisWorkbook.Sheets(2). Run Time error 9 subscript out of range. I assumed the top part was a Sub and the bottom part a Function. Can you let me know what I've done wrong. Is your code better because it stores the count on the computer and fills in the range in one go as opposed to my code that jumps from sheet to sheet after each row of code? I tried to copy the code in here but it's too long, would you be able to re-send the code with a dummy sub before and after your code so I can see how it should look. Sorry, but I'm a complete novice, cheers – Cobber Oct 31 '19 at 00:05
Can you share a copy of your workbook? If you're totally new to VBA then I'm not going to be able to help piece-by-piece: there's just too much I'd have to spell out. – Tim Williams Oct 31 '19 at 00:25
You can host it on dropbox/box/gdrive/onedrive etc and share the link – Tim Williams Oct 31 '19 at 02:27
That's on your computer – Tim Williams Oct 31 '19 at 03:04
OK I will take a look in a little while – Tim Williams Oct 31 '19 at 03:22
There too much code there - 6k lines in a single module - I can't even make a dent on this. If you can point to one specific part I will make some suggestions. In general I see a whole bunch of repeated operations such as setting the same set of column widths - you need to factor out code like that into standalone procedures and pass in specific ranges as arguments. That's the only way this is going to be maintainable in the long run. Also a bunch of "select a range/operate on selection" which could just be "operate on range" similar to my second comment on your question. – Tim Williams Oct 31 '19 at 03:46
A lot of your code could be replaced by using some template worksheets which you can just copy as needed - no need to code all that setup. – Tim Williams Oct 31 '19 at 03:47
I wouldn't say it will crash - it will likely work fine but it will be a bit of a nightmare if you need to change anything. – Tim Williams Oct 31 '19 at 04:08
Sorry I can't do anything with this which would be useful to you - if your VBA is early-level then anything I write will be over-complex for your needs going forward. – Tim Williams Oct 31 '19 at 04:14
No worries....if it isn't going to crash that's all I need to know. Thanks for your help and patience, you have been a great help. Cheers. – Cobber Oct 31 '19 at 04:28
Sorry I can't help - that's just too much code to go through. Are you sure you wouldn't be better off using a pivot table or two for this? You seem to be mostly counting/summarizing data, and a pivot table would be a good fit. – Tim Williams Nov 05 '19 at 22:27
No problem - go ahead. If you can boilt it down to a *much* smaller set of code and explain exactly what the error is that would help. – Tim Williams Nov 05 '19 at 23:06

score 0 · Answer 2 · answered Oct 30 '19 at 06:58

My suggestions to you will be:

If you are using MS Excel 2010 and above, you could create Pivot tables. Glancing through your code, I understand that you creating measures based on certain values for a given range. Pivot tables handle this scenario better.

If you believe you will need to use this code, then you might need to look at using Named ranges. Named Ranges are used for readability and software maintainability.

For example, you could define a name for column range C7:C10000 as nm_SomeDescription. The keyboard shortcut to define a name is CTRL+F3 or you could navigate to Formulas | Name Manager The VBA code will use the name defined instead of column range as in

Sheets(1).Range("F9").Value = Application.WorksheetFunction.CountIfs(Range(nm_SomeDescription), nm_HUJCode, Range(nm_SewerDescription), nm_SEWERCODE, Range(nm_FailureDescription), nm_FAILCODE)

CountIF or CountIFs use sequential scanning methods. Hence, you might see a performance hit for a large dataset. To mitigate the performance, you could sort the data before calling the CountIFS function.

Hi Devasuran, do you mean just put the header name of the column like this. PS, how do you get this code in grey background?? Sheets(1).Range("E5").Value = Application.WorksheetFunction.CountIfs(Range(Area), "RDG1", Range(Work Type), "Sewer Works", Range(Result), "PASS") — Cobber, Oct 30 '19 at 07:23
Given the situation, please follow what @Tim Williams says. Also, consider defining variable names using Named Manager. Define the name as per your business rules. The names which I have used is only for Example. To get the code block in grey, use the markdown formatting tags `code block here` — devasuran, Oct 30 '19 at 07:48

Is this countifs VBA code OK, or will it crash?

2 Answers2