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

Question

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)

Answer 1

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!

Answer 2

YES

I translated it from your case statement. I find it easier to read, personally.

If Item Is Nothing Then
  Exit Sub ' we got a non-element
ElseIf LastSelected Is Nothing Then
  Set LastSelected = Item ' we got our first good element
ElseIf Item = LastSelectedItem Then
  Exit Sub ' we already had what we got
Else
  Set LastSelected = Item ' we got something new
End If

You asked for explanation. I tried not to have to give much (by re-using your own code comments).

But here it is anyway :-)

• Firstly if there is no item, just exit. Easy.
• Otherwise if LastSelected Is Nothing then we know, because the first if condition failed, that Item exists, and it's safe to mark that value as having been Last Selected. As you say, we got our first good element. The sub continues on.
• However if we have existing values for Item and LastSelected, then either they are equal or not. If they are equal, just quit.
• If they aren't equal, then update LastSelected. As you say, we got something new.

Answer 3

You can use a helper function like this:

Private Function pvGetItemData(oItem As ListItem) As Variant
    If Not oItem Is Nothing Then
        pvGetItemData = oItem.Tag
    Else
        pvGetItemData = -1
    End If
End Function

and then

If pvGetItemData(Item) = pvGetItemData(LastSelected) Then
    ' cache hit
Else
    ' do calc
    Set LastSelected = Item
End If

Answer 4

YES

I'd make that simpler:

If item Is Nothing Then
  Exit Sub ' we got a non-element
Else
  Set lastSelected = item ' we got something to assign
End If

Unless there are side effects assigning lastItem (it can be property with invalid assignment code), then code logic is essentially same.

If you are not required to exit the sub (snippet is at the end of sub or something), then next is even simpler:

If Not (item Is Nothing) Then Set lastSelected = item

BTW, your Select Case (True) looks really odd to VB programmer :)