Excel VBA If And Then Not Evaluating Properly

Question

I have the below code, and there appears to me to be a problem with the IF statements. There are two tabs in the workbook, the ALLDATA tab and the COMP tab. The macro is supposed to populate a list of payees (row 1 of the ALLDATA tab), by date (column A of the ALLDATA tab), by amount (separate columns on the COMP tab, headed by row 1). It is supposed to skip cells that have a 0 or blank value. All cells with data will have positive values, which is why I am checking > 0. Here is a sample what the ALLDATA tab looks like:

This is what I would expect the output to look like on the COMP tab:

Here is what the actual output looks like:

And here is my code:

Sub Payee_Date()
'
' Payee_Date Macro
'
Application.ScreenUpdating = False

'Select the COMP tab
Sheets("COMP").Activate

'Find the last row of the COMP tab
Dim compLastRow As Long
compLastRow = Cells(Rows.Count, 1).End(xlUp).Row

'Clear the COMP tab
With Sheets("COMP")
    .Rows(2 & ":" & compLastRow).Delete
End With

'Select the ALLDATA tab
Sheets("ALLDATA").Activate

'Find the last row of the ALLDATA tab
Dim alldataLastRow As Long
alldataLastRow = Cells(Rows.Count, 1).End(xlUp).Row

'Find the last column of the ALLDATA tab
Dim alldataLastColumn As Long
With ActiveSheet
    alldataLastColumn = .Cells(1, .Columns.Count).End(xlToLeft).Column
End With

'Loop through the sheet and populate the COMP tab
Dim alldataColNum As Integer
Dim alldataRowNum As Integer
Dim compRowNum As Long
Dim payee As String
Dim day As String
Dim amount As Double

compRowNum = 2

For alldataColNum = 2 To alldataLastColumn Step 1
    For alldataRowNum = 2 To alldataLastRow Step 1
        If Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value > 0 And   IsEmpty(Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value2) = False Then _
            amount = Cells(alldataRowNum, alldataColNum).Value2
            payee = Cells(1, alldataColNum).Value2
            day = Cells(alldataRowNum, 1).Value2

                Sheets("COMP").Activate
                Range("A" & compRowNum).Value = day
                Range("B" & compRowNum).Value = payee
                Range("C" & compRowNum).Value = amount
                Sheets("ALLDATA").Activate

                compRowNum = compRowNum + 1
    Next alldataRowNum
Next alldataColNum

Application.ScreenUpdating = True

'
End Sub

Any help with identifying my flawed logic/code would be greatly appreciated.

Answer 1

Logical lines

The VBA specs defines a concept of logical code lines.

This spans 4 lines in a code module:

If _
    True _
        Then _
            MsgBox "Hi!"

But as far as VBA is concerned, it's one single logical line of code; the statement is parsed and interpreted like this:

If True Then MsgBox "Hi!"

Which is entirely valid VBA: the underscore character is parsed as a line continuation token, and VBA interprets it as being on the same logical line.

Works for comments too (although it's debatable whether that's a good thing):

'This comment starts here _
         ...and continues there

---

Inline vs. Block conditions

The conditional If statement supports two syntaxes:

• Inline syntax, as above:

  If True Then MsgBox "Hi!"
  

• Block syntax:

  If True Then
      MsgBox "Hi!"
  End If
  

When using the block syntax, every statement between the Then keyword and the End If token will execute conditionally, per the specified condition.

If any statement follows the Then keyword on the same logical line of code, VBA attempts to parse the instruction as the inline syntax; if an {EndOfLineInstruction} token (a new line, basically) follows the Then keyword, VBA will be parsing the conditional as the block syntax, and will expect to encounter an End If token before it reaches the end of the current scope (e.g. an End Sub token). If there's no End If token, VBA throws a compile error.

---

In your case, you have:

If someCondition Then _
    DoSomething
    DoSomethingElse

Which parses as:

If someCondition Then DoSomething
DoSomethingElse

But what you intended was:

If someCondition Then
    DoSomething
    DoSomethingElse
End If

Answer 2

You have a very subtle bug introduced by the line-continuation character:

If Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value > 0 And   IsEmpty(Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value2) = False Then **_**

That continuation character causes the "then" portion to execute the next statement if the test condition evaluates to true, and immediately terminate the IF block. Control continues, executing all lines following it - the ones you intend to be controlled by the 'if/then'. Take off that trailing "_" and add an "End If" just before the innermost Next and it should resolve your problem.

For alldataRowNum = 2 To alldataLastRow Step 1
    If Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value > 0 And   IsEmpty(Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value2) = False Then 
        amount = Cells(alldataRowNum, alldataColNum).Value2
        payee = Cells(1, alldataColNum).Value2
        day = Cells(alldataRowNum, 1).Value2

            Sheets("COMP").Activate
            Range("A" & compRowNum).Value = day
            Range("B" & compRowNum).Value = payee
            Range("C" & compRowNum).Value = amount
            Sheets("ALLDATA").Activate

            compRowNum = compRowNum + 1
   End If ' Add the IF block closure here
Next alldataRowNum

Answer 3

Try to replace this line:

If Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value > 0 And   IsEmpty(Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value2) = False Then _

with:

If IsNumeric(Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum)) Then 

As you've used "-" where there's no numbers, I think that checking if a value is numeric is sufficient.

Answer 4

Part of your conditional is always testing true and since you are storing the values from previous iterations it is repeating it for each test. Your condition:

IsEmpty(Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value2) = False

Should be:

Sheets("ALLDATA").Cells(alldataRowNum, alldataColNum).Value2 = ""

This will catch empty cells and skip them.