What's so bad about using button captions as variables in VB6?

Question

I received some justified critical feedback on my last question (How to gracefully exit from the middle of a nested subroutine when user cancels?) for using the caption of a command button as a state variable. I did it because it's efficient, serving two or three purposes at once with very little code, but I understand how it could also cause problems, particularly in the slightly sloppy way I originally presented it.

I feel like this deserves its own discussion, so here's the same idea cleaned up a bit and modified to do it "right" (which basically means defining the strings in a single place so your code won't start failing because you simply changed the text of a command button). I know my variable and control naming convention is poor (OK, nonexistent), so apologies in advance. But I'd like to stay focused on the caption as state variable discussion.

So here we go:

' Global variables for this form
Dim DoTheThingCaption(1) As String
Dim UserCancel, FunctionCompleted As Boolean

Private Sub Form_Initialize()
  ' Define the possible captions (is there a #define equivalent for strings?)
  DoTheThingCaption(0) = "Click to Start Doing the Thing"
  DoTheThingCaption(1) = "Click to Stop Doing the Thing"

  ' Set the caption state when form initializes
  DoTheThing.Caption = DoTheThingCaption(0)
End Sub

Private Sub DoTheThing_Click() ' Command Button

If DoTheThing.Caption = DoTheThingCaption(0) Then
  UserCancel = False ' this is the first time we've entered this sub
Else ' We've re-entered this routine (user clicked on button again
     ' while this routine was already running), so we want to abort
  UserCancel = True ' Set this so we'll see it when we exit this re-entry
  DoTheThing.Enabled = False 'Prevent additional clicks
  Exit Sub
End If

' Indicate that we're now Doing the Thing and how to cancel
DoTheThing.Caption = DoTheThingCaption(1)

For i = 0 To ReallyBigNumber
  Call DoSomethingSomewhatTimeConsuming
  If UserCancel = True Then Exit For ' Exit For Loop if requested
  DoEvents ' Allows program to see GUI events
Next

' We've either finished or been canceled, either way
' we want to change caption back
DoTheThing.Caption = DoTheThingCaption(0)

If UserCancel = True Then GoTo Cleanup

'If we get to here we've finished successfully
FunctionCompleted = True
Exit Sub '******* We exit sub here if we didn't get canceled ******* 

Cleanup:
'We can only get to here if user canceled before function completed

FunctionCompleted = False
UserCancel = False ' clear this so we can reenter later
DoTheThing.Enabled = True 'Prevent additional clicks

End Sub  '******* We exit sub here if we did get canceled ******* 

So there it is. Is there still anything really that bad about doing it this way? Is it just a style issue? Is there something else that would give me these four things in a more desirable or maintainable way?

1. Instant GUI feedback that user's button press has resulted in action
2. Instant GUI feedback in the location where user's eyes already are on how to CANCEL if action is not desired
3. A one-button way for users to start/cancel an operation (reducing the amount of clutter on the GUI)
4. A simple, immediate command button disable to prevent multiple close requests

I can see one concern might be the close coupling (in several ways) between the code and the GUI, so I could see how that could get to be a big problem for large projects (or at least large GUIs). This happens to be a smaller project where there are only 2 or 3 buttons that would receive this sort of "treatment".

Answer 1

The single biggest problem with this technique is that it uses a string as a boolean. By definition, a boolean variable can have only two states, while a string can have any number of states.

Now, you've mitigated the danger inherent in this somewhat by relying on an array of predefined strings to define allowed values for the command button text. This leaves a handful of lesser issues:

• Logic is less-than-explicit regarding current and available states (there are actually four possible states for the form: not-started, started, completed, started-but-canceling) - maintenance will require careful observation of the potential interactions between button text and boolean variable states to determine what the current state is / should be. A single enumeration would make these states explicit, making the code easier to read and understand, thereby simplifying maintenance.
• You're relying on the behavior of a control property (button text) to remain consistent with that of the exposed property value type (string). This is the sort of assumption that makes migrating old VB6 code to newer languages / platforms absolute hell.
• String comparison is much, much slower than a simple test of a boolean variable. In this instance, this won't matter. In general, it's just as easy to avoid it.
• You're using DoEvents to simulate multi-threading (not directly relevant to the question... but, ugh).