Reputation: 1178
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
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
.
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
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: 12520-Jun-2012 19:28:26
nested_ifs (x1): 156 - Last Run: 156 - Average Run: 156
or_and (x1): 125 - Last Run: 125 - Average Run: 12520-Jun-2012 19:28:27
nested_ifs (x1): 140 - Last Run: 140 - Average Run: 140
or_and (x1): 125 - Last Run: 125 - Average Run: 12520-Jun-2012 19:28:28
nested_ifs (x1): 140 - Last Run: 140 - Average Run: 140
or_and (x1): 141 - Last Run: 141 - Average Run: 14120-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
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