1

Got a bizarre issue in an IF statement with multiple conditions. The last working version looks like:

If VAR1 = "SCRT" And InStr("|TEC|FIN|", "|" & VAR2 & "|")) And VAR3 = 1 Then

And now, I implement another condition to statement:

If VAR1 = "SCRT" And InStr("|TEC|FIN|", "|" & VAR2 & "|")) And (VAR3 = 1 or VAR3 = 15) Then

With this values: VAR1: "" (empty) VAR2: "FIN" VAR3: 1

The code it will work fine in debug. The bizarre is compiling and running on the production environment, the condition passed even with the first condition not ok.

Already logged the values to check. The production environment has the same files (libraries).

The only thing that changed besides the condition is the Option Explicit in the begin of this module.

Thanks!!

Ňɏssa Pøngjǣrdenlarp
  • 38,411
  • 12
  • 59
  • 178
Fernando Moreno
  • 41
  • 5
  • `On Error Resume Next` and `VAR1` is null? – wqw Feb 23 '17 at 17:32
  • 2
    Instr returns the position of the match (either 1 or 5 or 0). The And operator is bitwise in VB6. A TRUE value is -1, so all bits are set. You should check for result of Instr being > 0. That will return either 0 or -1 and your expression will be more correct. – Guillermo Phillips Feb 23 '17 at 17:34
  • I saw this, but in this case vb6 implicitly convert any value >0 to True. – Fernando Moreno Feb 23 '17 at 19:45
  • 1
    Only by luck. If you had two Instr conditions for example, it may fail. This would fail: If InStr("abc", "a") And InStr("abc", "b") Then – Guillermo Phillips Feb 24 '17 at 09:40
  • Ok.... this is an change that it'll be changed! =) But it's not the resolution. – Fernando Moreno Feb 24 '17 at 21:00
  • You have mismatched parentheses. Otherwise it may or may not be a Possible duplicate of [Odd behavior with boolean if statement in VBA](http://stackoverflow.com/q/24684955/11683). – GSerg Feb 24 '17 at 21:46

1 Answers1

2

No offense, but that's just way too hard to parse, and for no good reason. When dealing with And conditions, you're far better off using nested Ifs, placing the least likely conditions first and the most expensive tests last where possible. This is much clearer, and almost certainly faster:

If VAR1 = "SCRT" Then
 If InStr("|TEC|FIN|", "|" & VAR2 & "|")) > 0 Then
  If (VAR3 = 1) or (VAR3 = 15) Then

This allows short-circuiting, which is otherwise unavailable in VB6. Code doesn't run faster just because it's on one line.

Two changes: test If Instr > 0 rather than just If Instr. Yes, VB will evaluate a successful test to True / False, but it will never evaluate the Not of that expression to False, which will someday bite you. Believe it. (-:

Second change: I used parens to force the order of precedence for the Or conditions. There are rules, but you could get something like If VAR3 = (1 Or VAR3) = 15 when you least expect it. Be explicit and there are no surprises.

Jim Mack
  • 1,070
  • 1
  • 7
  • 16
  • Checking > 0 is also better 'defensive' programming. Since it is more explicit some future programmer is less likely to make a change which breaks it. – StayOnTarget Feb 24 '17 at 12:26
  • I was simplifying, but this is only one of many conditions in a list of rules. The real function has a lot of elseif's and if I put one condition inside another, the code will messy... First change: I've already put the >0 ... =D Second change: didn't understand the condition `If VAR3 = (1 Or VAR3) = 15`. Why you put the parens like that? Thanks!! – Fernando Moreno Feb 24 '17 at 21:04
  • @FernandoMoreno - That was just an example of the kind of problem you might encounter if your understanding of the rules of precedence aren't the same as the compiler's. Look in the code block for my actual recommendation. Be explicit: add parens to force evaluation when there might be confusion -- it never hurts, – Jim Mack Feb 24 '17 at 21:32
  • 1
    Also, a tangle of `if`s and `elseif`s is usually a sign that you should be trying a different approach – Jim Mack Feb 24 '17 at 21:33
  • I´m still confused about If InStr("|TEC|FIN|", "|" & VAR2 & "|")): my confusion comes from ONE opening "(" and TWO closing ")" – nabuchodonossor Feb 27 '17 at 08:11
  • @nabuchodonossor - Good catch. I just pasted that from the OP's code but it's clearly an error and would never run, so it can't be what he actually tested. A good reason not to post air code when asking for help (-: – Jim Mack Feb 27 '17 at 12:44