Search code examples
vb6stylesshort-circuitingselect-case

Is there a better way to write the following VB6 snippet?


I work at $COMPANY and I'm helping maintain $LEGACY_APPLICATION. It's written in visual basic 6.

I was faced with doing an unpleasantly elaborate nested if statement due to the lack of VB6's ability to perform short circuit evaluations in if statements (which would simplify this a lot). I've tried AndAlso, but to no avail. Must be a feature added after VB6.

Some genius on SO somewhere pointed out that you can trick a select case statement into working like a short-circuiting if statement if you have the patience, so I tried that, and here's what I came up with:

Select Case (True) ' pretend this is an if-else statement
    Case (item Is Nothing): Exit Sub ' we got a non-element
    Case ((item Is Not Nothing) And (lastSelected Is Nothing)): Set lastSelected = item ' we got our first good element
    Case (item = lastSelected): Exit Sub ' we already had what we got
    Case (Not item = lastSelected): Set lastSelected = item ' we got something new
End Select

It's definitely a little unusual, and I had to make use of my fantastic whiteboard (which, by the way, is pretty much the most useful programming resource besides a computer) to make sure I had mapped all of the statements correctly.

Here's what's going on there: I have an expensive operation which I would like to avoid repeating if possible. lastSelected is a persistent reference to the value most recently passed to this calculation. item is the parameter that was just received from the GUI. If there has never been a call to the program before, lastSelected starts out as Nothing. item can be Nothing too. Additionally, if both lastSelected and item are the same something, skip the calculation.

If I were writing this in C++, I would write:

if (item == NULL || (lastSelected != NULL && item->operator==(*lastSelected))) return;
else lastSelected = item;

However, I'm not.

Question

How can I rewrite this to look better and make more sense? Upvotes will be awarded to answers that say either "YES and here's why: X, Y, Z" or "NO, and here's why not: X, Y, Z".

Edits

Fixed the C++ statement to match the VB6 one (they were supposed to be equivalent)


Solution

  • This is shorter and 100x more readable.

    EDIT Wug edited the code in MarkJ's original answer, into this:

    If (item Is Nothing)
        Then Exit Sub ' we got a non-element
    ElseIf (lastSelected Is Nothing) Then
        Set lastSelected = item ' we got our first go 
    ElseIf (item = lastSelected) Then
        Exit Sub ' we already had what we got
    End If
    Set lastSelected = item ' we got something new 
    

    Here's MarkJ's edit in response. One nested if, but only one Set. Seems neater to me.

    If (item Is Nothing) Then 
      Exit Sub ' we got a non-element 
    ElseIf Not (lastSelected Is Nothing) Then ' not our first go
      If (item = lastSelected) Then 
        Exit Sub ' we already had what we got 
      End If 
    End If
    Set lastSelected = item ' we got something new
    ' does stuff here? @Wug is that true?
    
    • To compare reference equality in VB6 use item Is LastSelected. Because item = lastSelected will probably evaluate the default properties in the objects and compare those instead!
    • Since brevity appears to be a goal, consider this. If you Exit Sub when condition X is True, you don't need to check X again later. It is False! Unless it changes its value in between evaluations (e.g. X is a function that checks the system clock). You were checking whether item was lastSelected, then whether it wasn't. And if item Is Nothing is False, do not bother to check whether item Is Not Nothing is True!
    • VB6 does not short circuit for backwards compatibility with ancient versions of Basic
    • Stop worrying that VB6 is not some other language and relax!