Place boolean function in if statement expression

Question

I'm maintaining a VB6 app. For boolean functions, the original authors store the return value in a boolean variable before checking the result in an If Statement. I prefer to place the function directly in the If Statement. Is this a matter of preference or am I missing a potential pitfall in my style?

Original Author Style

bReturn = IsThisFun(maintainingVb6)
If bReturn = True Then
    'You haven't used .NET
Else
    'blah
End If

My Style

If IsThisFun(maintainingVb6) Then
    'You haven't used .NET
Else
    'blah
End If

I'm not sure if there is a proper terminology for these different approaches which may have allowed me to miss a previous post on this topic.

Thanks

Answer 1

Do whatever is most readable and maintainable. In my opinion, your style meets that criteria 98% of the time.

Answer 2

Can't you just make a method that returns a type of bool instead?

ex:

public bool maintainingVB6() { return true; }

Answer 3

I don't ever like declaring unnecessary variables. It's a waste of resources, and doesn't make the code more readable. I much prefer your style.

Answer 4

Strictly speaking, it's a matter of style.

The rule of thumb is: what's more readable. Yours is probably to be preferred, because his code is redundant and most people prefer to remove redundancy. (The least he could do is remove the = True, which is totally redundant.)

If the boolean was a result of a longer function call, where the If Then was getting in the way, I would make the result a separate variable to make seeing the function call easier, but that's just me.

Answer 5

I believe the original author created a variable to be able to debug the application. He would put a breakpoint at line 2, and will see the value of bReturn by hovering on it. But since you can also hover on equal symbol or put two breakpoints inside If/EndIf statement, there is no need to create a variable just for this.

By the way, I partially agree with DOK. There is no need to create unnecessary variables. It's not at all a waste of resources, because compiler is clever enough to remove this variable, but creating a variable such this does not make a code more readable.

Answer 6

There's one possible pitfall in using either method (w/ or wo/ variable). You have to certain that the function returns an actual VB Boolean (an OLE VARIANTBOOL), not a C bool or any other type. If the function is written and exposed in all-VB6, this isn't an issue, but...

It's not uncommon to create Declares for external (usually API) functions that are prototyped as returning bool, using As Boolean for the return type. VB accepts that, but when a 'True' (from the function's POV) is returned it may be in an anomalous state: a VB 16-bit Boolean that contains the value 1, instead of the normal -1.

This can have strange follow-on effects, the most obvious being a test for If Not MyFunction(), which will compute (Not 1), or -2, instead of (Not -1), or 0, as expected. So you get the strange result of Not(True) = True

The best practice is to Declare external 'bool' functions As Long and CBool() the result.

Answer 7

Actually most often than not this is not a matter of preferences. The separate local var matters if the code is wrapped in On Error Resume Next.

If IsThisFun bombs out then bReturn does not get assigned and usually ends up being false. so the condition is not satisfied. In the second snippet if IsThisFun raises an exception then the execution gets inside the If true block.

For overcoming this I've seen numerous "non-sense" snippets like this:

On Error Resume Next
...
If Not IsThisFun(maintainingVb6) Then
Else
    ...
End If