tags:

views:

527

answers:

6

(I'm using VB6 but I imagine this comes up in most other languages.)

I've got a GUI button that calls a routine that takes a minute or two to complete. I want impatient users to be able to click on the button a second time to have it gracefully exit out of the routine at any point.

I used a static variable to make this work pretty well (see code below), but I'm cleaning up the project and I want to put the For/Next loop into its own function, since it's required in several different places in the project.

But doing that would break my static flag embedded in the for/next, so I need to make some changes. Before I do something hare-brained with public (global) variables, I thought I'd ask what other (smarter, perhaps actually CS educated) people have done when faced with this problem.

So basically my question is how do I replicate this:

Private Sub DoSomething_Click()

  Static ExitThisSub As Boolean ' Needed for graceful exit

  If DoSomething.Caption = "Click To Stop Doing Something" Then
    ExitThisSub = False ' this is the first time we've entered this sub
  Else ' We've re-entered this routine (user clicked on button to stop it)
    ExitThisSub = True ' Set this so we'll see it when we exit this re-entry
    Exit Sub '
  End If


  DoSomething.Caption = "Click To Stop Doing Something"

  For i = 0 To ReallyBigNumber
    Call DoingSomethingSomewhatTimeConsuming
    If ExitThisSub = True Then GoTo ExitThisSubNow
    DoEvents
  Next

  ' The next line was missing from my original example,
  ' prompting appropriate comments
  DoSomething.Caption = "Click To Do Something"

  Exit Sub

ExitThisSubNow:

  ExitThisSub = False ' clear this so we can reenter later
  DoSomething.Caption = "Click To Do Something"

End Sub

When I move the for/next loop to its own function?

I'm thinking I'll change ExitThisSub to a public variable QuitDoingSoManyLongCalculations that will exit the new for/next sub and then the DoSomething_Click in the same way.

But I always feel like an amateur (which I am) when I use global variables - is there a more elegant solution?

+2  A: 

You need some kind of shared variable so that your for loop and your button can communicate. I'd put the for loop (and its associated code) in a command object. My VB is rusty but I think you can declare Modules with their own 'global' variables and functions. You can move all the code into a module and just check the global variable as you do now.

My main concern with the code sample you've posted has nothing to do with user-cancelling but rather everything else: you check your running state by reading the button text instead of doing that the other way around (set the button text because of the running state, which should be stored in a variable); you use a GOTO to exit your for loop instead of a break (does VB have breaks?) and you put your cleanup code outside the normal flow, when it seems to me that it could be run regardless of whether the user cancelled or not.

Mr. Shiny and New
I know using the button text introduces some risk, but doing it the "right" way adds more code. I think the best of both worlds is defining the button text in the code and using that throughout, so if it changes; it changes everywhere.
Fred Hamilton
No Break command that I'm aware of, but you made me look it up and I should be using "Exit For" - thanks for that.The cleanup code comes after an "Exit Sub", so it can't accidentally be run. I did it that way because that's how I've seen others do it - never thought about if it was ideal or not.
Fred Hamilton
You can also use the tag property of the control.
RS Conley
Exit For and Exit DO are the two ways VB has exiting a loop.
RS Conley
@Fred: it looks like, in your sample, that cleanup code should run after the loop ends no matter what, or else the button text won't be reset. And using a constant to store the button text helps, but I still think the pattern is fundamentally wrong.
Mr. Shiny and New
Mr. S: When the emergency exit is not used, the "Exit Sub" below the "Next" statement will exit the sub and the cleanup code will not be touched. You're right about the button text not being reset in normal operation - I forgot to include that statement when I wrote the example - will edit to fix.
Fred Hamilton
+2  A: 

One possible alternative is to offload your heavy-work function to a new Thread. Then you can either directly kill that thread if the user wants to cancel, or you can send a message to the thread.

Toggling via button name, as you are doing above, is a pretty commonly seen trick though, and pretty safe if you only have a couple of button states.

Mike
Threading in VB6 is very complex if you want to do anything but pure calculation.
danbystrom
Thanks for backing me up on the button name toggling! :-)
Fred Hamilton
+2  A: 

I've always used a global boolean variable like bUserPressedCancel, along with DoEvents within a loop. Elegant, smelegant, it works.

I agree with Mr Shiny that testing against the value of a caption is not a great idea. If you change the text on the button in the designer, you'll break the code. Better not to rely on the wording of the text for your code to work.

MikeW
+3  A: 

Well you could declare the variable at module level in the forms as private. That is not a global but a module level variable. Then you could pass it to the function you create and check it in the function.

But be careful with the DoEvents. It basically means allow the windows message loop to process messages. This means that not only can the user click your button again, they can close the form and do other things. So when you are in this loop you'll need to set a module level variable anyway as you'll need to check for it in a QueryUnload of the form and in any event handlers.

You can also use the Tag property of the control itself to store a flag of sorts. But I don't consider that more elegant.

I also prefer to use two different buttons. Just hide one and show the other. That way your cancel code and run code are separated in different event handlers.

To expand on my answer here is some sample code that handles the unloading aspect. Here if you stop via the x it prompts you. If you kill via task manager, it dies gracefully.

Option Explicit

Private Enum StopFlag
   NotSet = 0
   StopNow = 1
   StopExit = 2
End Enum

Private m_lngStopFlag As StopFlag
Private m_blnProcessing As Boolean

Private Sub cmdGo_Click()

   Dim lngIndex As Long
   Dim strTemp As String

   m_lngStopFlag = StopFlag.NotSet
   m_blnProcessing = True

   cmdStop.Visible = True
   cmdGo.Visible = False

   For lngIndex = 1 To 99999999

      ' check stop flag
      Select Case m_lngStopFlag

         Case StopFlag.StopNow

            MsgBox "Stopping - Last Number Was " & strTemp
            Exit For

         Case StopFlag.StopExit

            m_blnProcessing = False
            End

      End Select

      ' do your processing
      strTemp = CStr(lngIndex)

      ' let message loop process messages
      DoEvents

   Next lngIndex

   m_lngStopFlag = StopFlag.NotSet
   m_blnProcessing = False
   cmdGo.Visible = True
   cmdStop.Visible = False

End Sub

Private Sub cmdStop_Click()

   m_lngStopFlag = StopFlag.StopNow

End Sub

Private Sub Form_Load()

   m_blnProcessing = False

End Sub

Private Sub Form_QueryUnload(Cancel As Integer, UnloadMode As Integer)

   Select Case UnloadMode

      Case vbFormControlMenu, vbFormCode

         If m_blnProcessing Then

            Cancel = True

            If MsgBox("Unload Attempted - Cancel Running Process?", vbOKCancel + vbDefaultButton1 + vbQuestion, "Test") = vbOK Then

               m_lngStopFlag = StopFlag.StopExit

            End If

         End If

      Case Else

         m_lngStopFlag = StopFlag.StopExit
         Cancel = True

   End Select

End Sub
Will Rickards
A: 

Works until you need to do localization or any other thing else that require the UI to be changed independently of the logic. I would use the Tag property or a private module level variable . Then you can vary the caption independently from the logic.

RS Conley
A: 

If it was me I'd use 2 buttons - one to GO and one to STOP. The STOP button is made visible when you click GO. The Click event for STOP simply hides itself - that's it.

Your loop can then simply check to see if the STOP button is still visible. If it's not, that means it was clicked and you should break out.

Your controls are static objects with form scope...

Scott Evernden