[vba] "And" and "Or" troubles within an IF statement

I'm trying to use "And" & "Or" within an If statement. I probably have my syntax wrong.

the result comes back false when the data should make it true. Here is the code:

ElseIf (origNum = "006260006" Or origNum = "30062600006") And creditOrDebit = "D" Then

'do things here

End If

-When I debug and come to this line it hops over it and doesn't enter in.

-origNum actually equals "006260006" and creditOrDebit = "D".

-so I'm assuming my "Or" statement isn't working.

-Hopefully this is a quick easy question. Thanks!

This question is related to vba excel

The answer is


I like assylias' answer, however I would refactor it as follows:

Sub test()

Dim origNum As String
Dim creditOrDebit As String

origNum = "30062600006"
creditOrDebit = "D"

If creditOrDebit = "D" Then
  If origNum = "006260006" Then
    MsgBox "OK"
  ElseIf origNum = "30062600006" Then
    MsgBox "OK"
  End If
End If

End Sub

This might save you some CPU cycles since if creditOrDebit is <> "D" there is no point in checking the value of origNum.

Update:

I used the following procedure to test my theory that my procedure is faster:

Public Declare Function timeGetTime Lib "winmm.dll" () As Long

Sub DoTests2()

  Dim startTime1 As Long
  Dim endTime1 As Long
  Dim startTime2 As Long
  Dim endTime2 As Long
  Dim i As Long
  Dim msg As String

  Const numberOfLoops As Long = 10000
  Const origNum As String = "006260006"
  Const creditOrDebit As String = "D"

  startTime1 = timeGetTime
  For i = 1 To numberOfLoops
    If creditOrDebit = "D" Then
      If origNum = "006260006" Then
        ' do something here
        Debug.Print "OK"
      ElseIf origNum = "30062600006" Then
        ' do something here
        Debug.Print "OK"
      End If
    End If
  Next i
  endTime1 = timeGetTime

  startTime2 = timeGetTime
  For i = 1 To numberOfLoops
    If (origNum = "006260006" Or origNum = "30062600006") And _
      creditOrDebit = "D" Then
      ' do something here
      Debug.Print "OK"
    End If
  Next i
  endTime2 = timeGetTime

  msg = "number of iterations: " & numberOfLoops & vbNewLine
  msg = msg & "JP proc: " & Format$((endTime1 - startTime1), "#,###") & _
       " ms" & vbNewLine
  msg = msg & "assylias proc: " & Format$((endTime2 - startTime2), "#,###") & _
       " ms"

  MsgBox msg

End Sub

I must have a slow computer because 1,000,000 iterations took nowhere near ~200 ms as with assylias' test. I had to limit the iterations to 10,000 -- hey, I have other things to do :)

After running the above procedure 10 times, my procedure is faster only 20% of the time. However, when it is slower it is only superficially slower. As assylias pointed out, however, when creditOrDebit is <>"D", my procedure is at least twice as fast. I was able to reasonably test it at 100 million iterations.

And that is why I refactored it - to short-circuit the logic so that origNum doesn't need to be evaluated when creditOrDebit <> "D".

At this point, the rest depends on the OP's spreadsheet. If creditOrDebit is likely to equal D, then use assylias' procedure, because it will usually run faster. But if creditOrDebit has a wide range of possible values, and D is not any more likely to be the target value, my procedure will leverage that to prevent needlessly evaluating the other variable.


This is not an answer, but too long for a comment.

In reply to JP's answers / comments, I have run the following test to compare the performance of the 2 methods. The Profiler object is a custom class - but in summary, it uses a kernel32 function which is fairly accurate (Private Declare Sub GetLocalTime Lib "kernel32" (lpSystemTime As SYSTEMTIME)).

Sub test()

  Dim origNum As String
  Dim creditOrDebit As String
  Dim b As Boolean
  Dim p As Profiler
  Dim i As Long

  Set p = New_Profiler

  origNum = "30062600006"
  creditOrDebit = "D"

  p.startTimer ("nested_ifs")

  For i = 1 To 1000000

    If creditOrDebit = "D" Then
      If origNum = "006260006" Then
        b = True
      ElseIf origNum = "30062600006" Then
        b = True
      End If
    End If

  Next i

  p.stopTimer ("nested_ifs")
  p.startTimer ("or_and")

  For i = 1 To 1000000

    If (origNum = "006260006" Or origNum = "30062600006") And creditOrDebit = "D" Then
      b = True
    End If

  Next i

  p.stopTimer ("or_and")

  p.printReport

End Sub

The results of 5 runs (in ms for 1m loops):

20-Jun-2012 19:28:25
nested_ifs (x1): 156 - Last Run: 156 - Average Run: 156
or_and (x1): 125 - Last Run: 125 - Average Run: 125

20-Jun-2012 19:28:26
nested_ifs (x1): 156 - Last Run: 156 - Average Run: 156
or_and (x1): 125 - Last Run: 125 - Average Run: 125

20-Jun-2012 19:28:27
nested_ifs (x1): 140 - Last Run: 140 - Average Run: 140
or_and (x1): 125 - Last Run: 125 - Average Run: 125

20-Jun-2012 19:28:28
nested_ifs (x1): 140 - Last Run: 140 - Average Run: 140
or_and (x1): 141 - Last Run: 141 - Average Run: 141

20-Jun-2012 19:28:29
nested_ifs (x1): 156 - Last Run: 156 - Average Run: 156
or_and (x1): 125 - Last Run: 125 - Average Run: 125

Note

If creditOrDebit is not "D", JP's code runs faster (around 60ms vs. 125ms for the or/and code).