Search code examples
excelworksheet-functionvba

WorksheetFunction.CountA() is returning 0 when i dont think it should be


Basically i need to get a list of each different type of issue from the first sheet and then display on the second.

The first sheet has 1 row which is just titles for the columns and then 4 rows with data.

It was wokring but I think i accidently changed something i cant work out whats wrong now, if someone can see an issue or if theres a better way of doing this then im all ears.

Sub ListQueryTypes()

'Create Variables'
Dim numberOfIssues As Integer
Dim numberOfMacroIssues As Integer
Dim numberOfReportIssues As Integer
Dim numberOfTechnicalIssues As Integer
Dim numberOfTrendIssues As Integer
Dim cellValue As String

'Set query register as active worksheet'
Sheets("Query Register").Activate

'set range for search'
Set myRange = Range("G:G")

'Minus 1 for the first row being column name'
numberOfIssues = Application.WorksheetFunction.CountA(myRange)

'Do, for as many issues as were found previously'
For i = 2 To numberOfIssues + 1
    cellValue = ActiveSheet.Cells(i, 7).Value
        Select Case cellValue
            Case "Macro"
                numberOfMacroIssues = numberOfMacroIssues + 1
            Case "Report"
                numberOfReportIssues = numberOfReportIssues + 1
            Case "Technical"
                numberOfTechnicalIssues = numberOfTechnicalIssues + 1
            Case "Trend"
                numberOfTrendIssues = numberOfTrendIssues + 1

        End Select
Next i
Sheets("Inventory").Activate
ActiveCell = Cells(2, 2)
ActiveCell.Value = numberOfMacroIssues
ActiveCell.Offset(1).Value = numberOfReportIssues
ActiveCell.Offset(2).Value = numberOfTechnicalIssues
ActiveCell.Offset(3).Value = numberOfTrendIssues

Solution

  • To collect all the issues mentioned in the comments, to improve your code. Have a look at the comments in the code

    Option Explicit 'force to declare all variables correctly (this way you don't forget any)
    
    Public Sub ListQueryTypes()
    
        'Create Variables'
        Dim numberOfIssues As Long 'was Integer but we can always use Long (only advantages) and Excel has more rows than Integer can handle
        Dim numberOfMacroIssues As Long 
        Dim numberOfReportIssues As Long 
        Dim numberOfTechnicalIssues As Long 
        Dim numberOfTrendIssues As Long 
        'Dim cellValue As String  'you don't need that variable see below
    
        'Set query register as active worksheet'
        'Sheets("Query Register").Activate 'instead of activate we define that sheet as variable which is more save
                                           'and we use Worksheets
        Dim WsQueryReg As Worksheet
        Set WsQueryReg = ThisWorkbook.Worksheets("Query Register")
    
        'set range for search'
        Dim myRange As Range 'we should to declare all variable properly
        Set myRange = WsQueryReg.Range("G:G") 'we need to define in which sheet the range is
    
        'Minus 1 for the first row being column name'
        numberOfIssues = Application.WorksheetFunction.CountA(myRange)
    
        'Do, for as many issues as were found previously'
        Dim i As Long 'declare every variable first
        For i = 2 To numberOfIssues + 1
            'cellValue = WsQueryReg.Cells(i, 7).Value 'no need to write this in a variable we can use it directly in Select Case
            Select Case WsQueryReg.Cells(i, 7).Value 'was cellValue before
                Case "Macro"
                    numberOfMacroIssues = numberOfMacroIssues + 1
                Case "Report"
                    numberOfReportIssues = numberOfReportIssues + 1
                Case "Technical"
                    numberOfTechnicalIssues = numberOfTechnicalIssues + 1
                Case "Trend"
                    numberOfTrendIssues = numberOfTrendIssues + 1
            End Select
        Next i
    
        'we can use with instead of activating a sheet and selecting a cell
        'this also specifies in which sheet and workbook the cell is
        With ThisWorkbook.Worksheets("Inventory").Cells(2, 2)
            .Value = numberOfMacroIssues
            .Offset(1).Value = numberOfReportIssues
            .Offset(2).Value = numberOfTechnicalIssues
            .Offset(3).Value = numberOfTrendIssues
        End With
    End Sub
    

    Instead of looping through your rows For i = 2 To numberOfIssues + 1 you could probably count Macro, Report, etc. like

    With WsQueryReg
        numberOfMacroIssues = Application.WorksheetFunction.CounfIf(.Range(.Cells(2, 7), .Cells(numberOfIssues + 1, 7)), "Macro")
        numberOfReportIssues  = Application.WorksheetFunction.CounfIf(.Range(.Cells(2, 7), .Cells(numberOfIssues + 1, 7)), "Report")
        '… others here
    End With