Mike Kellogg
Mike Kellogg

Reputation: 1178

"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!

Upvotes: 19

Views: 289868

Answers (3)

JimmyPena
JimmyPena

Reputation: 8754

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.

Upvotes: 1

assylias
assylias

Reputation: 328568

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).

Upvotes: 3

assylias
assylias

Reputation: 328568

The problem is probably somewhere else. Try this code for example:

Sub test()

  origNum = "006260006"
  creditOrDebit = "D"

  If (origNum = "006260006" Or origNum = "30062600006") And creditOrDebit = "D" Then
    MsgBox "OK"
  End If

End Sub

And you will see that your Or works as expected. Are you sure that your ElseIf statement is executed (it will not be executed if any of the if/elseif before is true)?

Upvotes: 30

Related Questions