Nick
Nick

Reputation: 372

Excel VBA Variable is not incrementing correctly

I have the below Excel 2016 VBA function where I am outputting data to various worksheets. I seem to be having an issue with the data on my "Error" worksheet where I am getting a lot of blank rows between other rows with data.

This issue is only occuring with the "Error" worksheet. I have tried putting the k = k + 1 variable around some and it has not helped. Is there any obvious place where I should (or shouldn't be) incrementing k that may fix the issue? Or perhaps l and j need to be changed?

Function PPDdate()

    Dim PPD_1_Date As Date
    Dim PPD_2_Date As Date
    Dim TSpot_Date As Variant
    Dim i As Long, j As Long, k As Long

    j = Worksheets("PPDCI").Range("A" & Rows.Count).End(xlUp).Row + 1
    l = Worksheets("CI").Range("A" & Rows.Count).End(xlUp).Row + 1
    k = Worksheets("Error").Range("A" & Rows.Count).End(xlUp).Row + 1

    For i = 2 To lstrow

        PPD_1_Date = Worksheets("Data").Range("AW" & i)
        PPD_2_Date = Worksheets("Data").Range("BA" & i)
        Entity = Worksheets("Data").Range("J" & i)
        Dept = Worksheets("Data").Range("M" & i)
        TSpot_Date = Worksheets("Data").Range("AS" & i)


        If PPD_1_Date > PPD_2_Date Then
            Worksheets("PPDCI").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
            Worksheets("PPDCI").Range("F" & j).Value = PPD_1_Date
            Worksheets("PPDCI").Range("G" & j).Value = Worksheets("Data").Range("AX" & i).Value
            Worksheets("PPDCI").Range("H" & j).Value = Worksheets("Data").Range("AZ" & i).Value
            Worksheets("PPDCI").Range("I" & j).Value = Worksheets("Data").Range("AY" & i).Value
            Worksheets("PPDCI").Range("J" & j).Value = "1st IF STATEMENT"
            j = j + 1
        Else
            If PPD_1_Date < PPD_2_Date Then
                Worksheets("PPDCI").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                Worksheets("PPDCI").Range("F" & j).Value = PPD_2_Date
                'Worksheets("PPDCI").Range("G" & j).Value = "ELSE IF CONDITION"
                Worksheets("PPDCI").Range("G" & j).Value = Worksheets("Data").Range("BB" & i).Value
                Worksheets("PPDCI").Range("H" & j).Value = Worksheets("Data").Range("BD" & i).Value
                Worksheets("PPDCI").Range("I" & j).Value = Worksheets("Data").Range("BC" & i).Value
                Worksheets("PPDCI").Range("J" & j).Value = "2nd IF STATEMENT"
                j = j + 1

                'If IsEmpty(Worksheets("Data").Range(PPD_1_Date & i).Value) = True And IsEmpty(Worksheets("Data").Range(PPD_2_Date & i).Value) = True Then
                'GoTo EmptyRange
                'Else

            Else
                If Len(TSpot_Date) <> 0 And InStr(1, UCase(Worksheets("Data").Range("AT" & i).Value), "NEG") > 0 Then
                    Worksheets("CI").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                    Worksheets("CI").Range("D" & j).Value = "TSNG"
                    Worksheets("CI").Range("E" & j).Value = TSpot_Date
                    Worksheets("CI").Range("F" & j).Value = "3rd IF STATEMENT"
                    'j = j + 1

                Else
                    If Len(TSpot_Date) <> 0 And InStr(1, UCase(Worksheets("Data").Range("AT" & i).Value), "POS") > 0 Then
                        Worksheets("CI").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                        Worksheets("CI").Range("D" & j).Value = "TSPS"
                        Worksheets("CI").Range("E" & j).Value = TSpot_Date
                        Worksheets("CI").Range("F" & j).Value = "4th IF STATEMENT"
                        l = l + 1

                    Else
                        If (InStr(1, Entity, "CNG Hospital") > 0 Or InStr(1, Entity, "Home Health") > 0 Or InStr(1, Entity, "Hospice") > 0 Or InStr(1, Dept, "Volunteers") > 0 And (PPD_1_Date = 0 And PPD_2_Date = 0)) And TSpot_Date = 0 Then
                            Worksheets("Error").Range("A" & k & ":H" & k).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                            Worksheets("Error").Range("F" & k).Value = "REVIEW PPD DATA"
                            Worksheets("Error").Range("G" & k).Value = "5th IF STATEMENT"
                            k = k + 1

                        Else
                            Worksheets("Error").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                            Worksheets("Error").Range("D" & j).Value = TSpot_Date
                            Worksheets("Error").Range("E" & j).Value = Worksheets("Data").Range("AT" & i).Value
                            Worksheets("Error").Range("F" & j).Value = "REVIEW PPD/TSPOT DATA"
                            Worksheets("Error").Range("G" & j).Value = "6th IF STATEMENT"
                            'k = k + 1

                        End If

                    End If

                End If

            End If

        End If

'EmptyRange:
        'k = k + 1

    Next i

End Function

Example output:

enter image description here

Thanks in advance!

Upvotes: 0

Views: 413

Answers (4)

Mathieu Guindon
Mathieu Guindon

Reputation: 71177

As was noted in other answers, & j should have been & k. However that isn't the "root issue".

The "root issue" is poor variable naming. Take the time to name things properly, it's worth it.

  • Rename i to currentRowData
  • Rename j to currentRowPPDCI
  • Rename k to currentRowError
  • Rename l to currentRowCI

Writing code is 5% of the job. Reading code is the other 95%. Therefore, code should be written to be read, not just executed. Yes, using meaningful identifiers take - oh, a whole second more to type. But they also make the code easier to read, which makes bugs like these much easier to catch.

i is a fine identifier when you're looking at a trivial For...Next loop. Anything else deserves a real name.

l is arguably the single most horrible single-letter identifier to use, because it's easily mistaken for a 1 at a glance.

Upvotes: 1

cybernetic.nomad
cybernetic.nomad

Reputation: 6368

You are not consistent with the use of your row counters (j, k and l) in relation to your sheets (PPDCI, CI and Error)

try:

Function PPDdate()

    Dim PPD_1_Date As Date
    Dim PPD_2_Date As Date
    Dim TSpot_Date As Variant
    Dim i As Long, j As Long, k As Long

    j = Worksheets("PPDCI").Range("A" & Rows.Count).End(xlUp).Row + 1
    l = Worksheets("CI").Range("A" & Rows.Count).End(xlUp).Row + 1
    k = Worksheets("Error").Range("A" & Rows.Count).End(xlUp).Row + 1

    For i = 2 To lstrow

        PPD_1_Date = Worksheets("Data").Range("AW" & i)
        PPD_2_Date = Worksheets("Data").Range("BA" & i)
        Entity = Worksheets("Data").Range("J" & i)
        Dept = Worksheets("Data").Range("M" & i)
        TSpot_Date = Worksheets("Data").Range("AS" & i)


        If PPD_1_Date > PPD_2_Date Then
            Worksheets("PPDCI").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
            Worksheets("PPDCI").Range("F" & j).Value = PPD_1_Date
            Worksheets("PPDCI").Range("G" & j).Value = Worksheets("Data").Range("AX" & i).Value
            Worksheets("PPDCI").Range("H" & j).Value = Worksheets("Data").Range("AZ" & i).Value
            Worksheets("PPDCI").Range("I" & j).Value = Worksheets("Data").Range("AY" & i).Value
            Worksheets("PPDCI").Range("J" & j).Value = "1st IF STATEMENT"
            j = j + 1
        Else
            If PPD_1_Date < PPD_2_Date Then
                Worksheets("PPDCI").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                Worksheets("PPDCI").Range("F" & j).Value = PPD_2_Date
                'Worksheets("PPDCI").Range("G" & j).Value = "ELSE IF CONDITION"
                Worksheets("PPDCI").Range("G" & j).Value = Worksheets("Data").Range("BB" & i).Value
                Worksheets("PPDCI").Range("H" & j).Value = Worksheets("Data").Range("BD" & i).Value
                Worksheets("PPDCI").Range("I" & j).Value = Worksheets("Data").Range("BC" & i).Value
                Worksheets("PPDCI").Range("J" & j).Value = "2nd IF STATEMENT"
                j = j + 1
            Else
                If Len(TSpot_Date) <> 0 And InStr(1, UCase(Worksheets("Data").Range("AT" & i).Value), "NEG") > 0 Then
                    Worksheets("CI").Range("A" & l & ":C" & l).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                    Worksheets("CI").Range("D" & l).Value = "TSNG"
                    Worksheets("CI").Range("E" & l).Value = TSpot_Date
                    Worksheets("CI").Range("F" & l).Value = "3rd IF STATEMENT"
                    l = l + 1
                Else
                    If Len(TSpot_Date) <> 0 And InStr(1, UCase(Worksheets("Data").Range("AT" & i).Value), "POS") > 0 Then
                        Worksheets("CI").Range("A" & l & ":C" & l).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                        Worksheets("CI").Range("D" & l).Value = "TSPS"
                        Worksheets("CI").Range("E" & l).Value = TSpot_Date
                        Worksheets("CI").Range("F" & l).Value = "4th IF STATEMENT"
                        l = l + 1
                    Else
                        If (InStr(1, Entity, "CNG Hospital") > 0 Or InStr(1, Entity, "Home Health") > 0 Or InStr(1, Entity, "Hospice") > 0 Or InStr(1, Dept, "Volunteers") > 0 And (PPD_1_Date = 0 And PPD_2_Date = 0)) And TSpot_Date = 0 Then
                            Worksheets("Error").Range("A" & k & ":H" & k).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                            Worksheets("Error").Range("F" & k).Value = "REVIEW PPD DATA"
                            Worksheets("Error").Range("G" & k).Value = "5th IF STATEMENT"
                            k = k + 1
                        Else
                            Worksheets("Error").Range("A" & k & ":C" & k).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                            Worksheets("Error").Range("D" & k).Value = TSpot_Date
                            Worksheets("Error").Range("E" & k).Value = Worksheets("Data").Range("AT" & i).Value
                            Worksheets("Error").Range("F" & k).Value = "REVIEW PPD/TSPOT DATA"
                            Worksheets("Error").Range("G" & k).Value = "6th IF STATEMENT"

                        End If
                    End If
                End If
            End If
        End If
    Next i

End Function

Upvotes: 0

oxwilder
oxwilder

Reputation: 767

It looks to me like your final "else" clause is using the last row of the "PPDCI" worksheet instead of the "errors" worksheet--the variable j instead of k.

Upvotes: 0

learnAsWeGo
learnAsWeGo

Reputation: 2282

You are setting the range in the else statement of your print to ERROR sheet with J when it should be K

                Else
                    If (InStr(1, Entity, "CNG Hospital") > 0 Or InStr(1, Entity, "Home Health") > 0 Or InStr(1, Entity, "Hospice") > 0 Or InStr(1, Dept, "Volunteers") > 0 And (PPD_1_Date = 0 And PPD_2_Date = 0)) And TSpot_Date = 0 Then
                        Worksheets("Error").Range("A" & k & ":H" & k).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                        Worksheets("Error").Range("F" & k).Value = "REVIEW PPD DATA"
                        Worksheets("Error").Range("G" & k).Value = "5th IF STATEMENT"
                        k = k + 1

                    Else
                        Worksheets("Error").Range("A" & j & ":C" & j).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                        Worksheets("Error").Range("D" & j).Value = TSpot_Date
                        Worksheets("Error").Range("E" & j).Value = Worksheets("Data").Range("AT" & i).Value
                        Worksheets("Error").Range("F" & j).Value = "REVIEW PPD/TSPOT DATA"
                        Worksheets("Error").Range("G" & j).Value = "6th IF STATEMENT"
                        k = k + 1

                    End If

Should be

            Else
                If (InStr(1, Entity, "CNG Hospital") > 0 Or InStr(1, Entity, "Home Health") > 0 Or InStr(1, Entity, "Hospice") > 0 Or InStr(1, Dept, "Volunteers") > 0 And (PPD_1_Date = 0 And PPD_2_Date = 0)) And TSpot_Date = 0 Then
                    Worksheets("Error").Range("A" & k & ":H" & k).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                    Worksheets("Error").Range("F" & k).Value = "REVIEW PPD DATA"
                    Worksheets("Error").Range("G" & k).Value = "5th IF STATEMENT"
                    k = k + 1

                Else
                    Worksheets("Error").Range("A" & k & ":C" & k).Value = Worksheets("Data").Range("A" & i & ":C" & i).Value
                    Worksheets("Error").Range("D" & k).Value = TSpot_Date
                    Worksheets("Error").Range("E" & k).Value = Worksheets("Data").Range("AT" & i).Value
                    Worksheets("Error").Range("F" & k).Value = "REVIEW PPD/TSPOT DATA"
                    Worksheets("Error").Range("G" & k).Value = "6th IF STATEMENT"
                    k = k + 1
                End If

I think that same issue can be found in your print to CI else if statement.

Also, as one of the comments states, how are you setting your last row?

You should consider declaring your worksheets, would make this cleaner IMO

Upvotes: 3

Related Questions