0

I'm trying to write a code to count the number of instances where a certain date AND a particular string occurs together in a database. I've written the following For Loop. However, the output for "JanHousekeeping" returns a value of 15.

The set of sample data that I am using looks like this.

enter image description here

I have 3 instances of "Housekeeping and other Hazards" but the loop returns a value of 15. Why is this and how could I fix this? Thank you in advance for the help!

Sub SummarySync()


  JanHousekeeping = 0
  FebHousekeeping = 0
  MarHousekeeping = 0
  AprHousekeeping = 0
  MayHousekeeping = 0
  JunHousekeeping = 0
  JulHousekeeping = 0
  AugHousekeeping = 0
  SepHousekeeping = 0
  OctHousekeeping = 0
  NovHousekeeping = 0
  DecHousekeeping = 0

  For i = 5 To 20
    For j = 5 To 20
      DateCheckLoop = Worksheets("Jan").Cells(i, 4)
      ObsCheck = Worksheets("Jan").Cells(j, 9).Value

       If Month(DateCheckLoop) = 1 And ObsCheck = "Housekeeping and Other Hazards" Then
          JanHousekeeping = JanHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 2 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          FebHousekeeping = FebHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 3 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          MarHousekeeping = MarHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 4 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          AprHousekeeping = AprHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 5 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          MayHousekeeping = MayHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 6 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          JunHousekeeping = JunHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 7 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          JulHousekeeping = JulHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 8 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          AugHousekeeping = AugHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 9 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          SepHousekeeping = SepHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 10 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          OctHousekeeping = OctHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 11 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          NovHousekeeping = NovHousekeeping + 1
       ElseIf Month(Worksheets("Jan").Cells(i, 4)) = 12 And Worksheets("Jan").Cells(j, 9).Value = "Housekeeping and Other Hazards" Then
          DecHousekeeping = DecHousekeeping + 1
       Else
       End If
     Next j
   Next i

   Sheets("Site Visit Summary").Range("DateJan").Offset(1, 0) = JanHousekeeping       
End Sub
3
  • What does stepping through the code using the debugger tell you? Commented Feb 20, 2019 at 3:04
  • If your first if statement is true when j=5 then it will count every time your outer i loop increments and j goes back to 5 thus giving you a total of 15. It appears your double loop is flawed as you are cycling through rows for both i and j Commented Feb 20, 2019 at 3:11
  • Does it have to be VBA? =SUMPRODUCT((MONTH(D5:D20)=1)*(I5:I20="Housekeeping and Other Hazards")) Commented Feb 20, 2019 at 3:49

2 Answers 2

1

I think this is what you mean to do although I would opt for the pivot table solution since pivots are dynamic by nature which is good in the long run.


  1. Your question implies that the criteria needs to be met for a single row but your loop is comparing your first date to all other rows for type. I believe you just want to increment the rows in unison. The easiest way to do that is just use one counter (i) to loop through the rows.
  2. If the Housekeeping and Other Hazards is a test that must be met for all of your inputs, just test that first outside of the loop.
  3. If you declare your variables properly, they will default to 0 and, more importantly, you will have more explicit code
  4. Select Case works nicely here although an array solution would be better and this could be condensed to a shorter code with loop

Sub Test()

Dim ws As Worksheet: Set ws = ThisWorkbook.Sheets("Jan")
Dim i As Long

Dim Jan As Long, Feb As Long, Mar As Long, Apr As Long, May As Long, Jun As Long, _
    Jul As Long, Aug As Long, Sep As Long, Oct As Long, Nov As Long, Dec As Long

For i = 5 To 20
    If ws.Range("I" & i) = "Housekeeping and Other Hazards" Then
        Select Case Month(ws.Range("D" & i))
            Case 1
                Jan = Jan + 1
            Case 2
                Feb = Feb + 1
            Case 3
                Mar = Mar + 1
            Case 4
                Apr = Apr + 1
            Case 5
                May = May + 1
            Case 6
                Jun = Jun + 1
            Case 7
                Jul = Jul + 1
            Case 8
                Aug = Aug + 1
            Case 9
                Sep = Sep + 1
            Case 10
                Oct = Oct + 1
            Case 11
                Nov = Nov + 1
            Case 12
                Dec = Dec + 1
        End Select
    End If
Next i

End Sub

A pivot table solution would look something like the below photo. If you swap the row fields, you can even filter by type (the current view lets you filter by date). You can use GETPIVOTDATA to set cells equal to certain conditions of the pivot table much like your macro aims to do at the end

enter image description here

Sign up to request clarification or add additional context in comments.

Comments

0

You are looping through j but using i to retrieve the date into DateCheckLoop.

You've provided no indication what i could be so determining the value at Worksheets("Jan").Cells(i, 4) is impossible. I can offer that the MONTH of anything between 1 and 31 will be 1.

This is repeated every loop as i is never changed.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.