Progress Bar Form

Question

Forms in MS Access are really just special glorified classes, so you can create new instances of them from code like this.

Dim someForm as Form_Form1
set someForm = New Form_Form1
someForm.visible = true

I decided to take advantage of this and create a progress bar form to display during long running processes. I created a form named ProgressBar with a rectangle and a text box.

• txtStatus
• boxProgress

progress bar form in design view

Form ProgressBar

Option Compare Database
Option Explicit
' Class: Form_ProgressBar
' Popup progress bar
Private Const MaxBoxWidth As Long = 7200
Public Enum ePBarModeType
 PBarMode_Percent = 0
 PBarMode_Executing = 1
End Enum
Private mMode As ePBarModeType
Private mCurrentProgress As Long
Private mSteps As Long
Public Property Get PercentComplete() As Double
'read only
 PercentComplete = mCurrentProgress / mSteps * 100
End Property
Public Property Let Mode(PBarMode As ePBarModeType)
 mMode = PBarMode
End Property
Public Property Get Mode() As ePBarModeType
 Mode = mMode
End Property
Public Property Let CurrentProgress(lng As Long)
' Updating the CurrentProgress property updates the status of the Progress Bar
 mCurrentProgress = lng
 ' format #0 makes a 1 or 2 digit number without decimals
 If mMode = PBarMode_Percent Then
 Me.txtStatus = Format(Me.PercentComplete, "#0") & " % Complete"
 ElseIf mMode = PBarMode_Executing Then
 Me.txtStatus = "Executing..."
 End If
 ' boxProgress.Width = a percentage of maximum box width
 Me.boxProgress.Width = (mCurrentProgress / mSteps) * MaxBoxWidth
 Me.Repaint
 DoEvents
End Property
Public Property Get CurrentProgress() As Long
 CurrentProgress = mCurrentProgress
End Property
Property Let steps(lng As Long)
 mSteps = lng
End Property
Public Sub init(steps As Long, Mode As ePBarModeType, Optional strCaption As String = "Loading...")
 Me.Mode = Mode
 Me.Caption = strCaption
 mCurrentProgress = 0
 mSteps = steps
 Me.txtStatus = "Ready"
 Me.boxProgress.Width = 0
 Me.Visible = True
End Sub

Example Calls

Private Sub exampleCall1()
' example call for using progress bar with a looping process
 Dim pbar As Form_ProgressBar
 Dim i As Long
 Dim steps As Long
 steps = 100000
 Set pbar = New Form_ProgressBar
 With pbar
 .init steps, PBarMode_Percent, "Hey, I'm working here!"
 For i = 1 To steps
 ' do something in a loop
 ' update progress
 .CurrentProgress = i
 Next i
 End With
 Set pbar = Nothing
End Sub
Private Sub exampleCall2()
' example call for using progress bar with an adodb.command
 Dim bimic As New BiMic_Wrapper
 Dim cmd As New ADODB.Command
 Dim prm As ADODB.Parameter
 DoCmd.HourGlass True
 With cmd
 .ActiveConnection = bimic.Connection
 .CommandType = adCmdStoredProc
 .CommandTimeout = 0
 .CommandText = "dbo.uspSomeStoredProcedure"
 End With
 'must execute command async
 cmd.Execute , , adAsyncExecute
 Dim pbar As New Form_ProgressBar
 Dim i As Long
 pbar.init 10000, PBarMode_Executing, ""
 Do While cmd.STATE = adStateExecuting
 For i = 0 To 10000
 pbar.CurrentProgress = i
 Next i
 Loop
 Set pbar = Nothing
 DoCmd.Hourglass False
End Sub

Concerns

• Am I really gaining anything from using a full fledged property for Mode? It's a simple Get & Let. Would it be cleaner to simply use a Public variable instead?
• Am I doing enough to ensure that the status doesn't display anything stupid? (Like displaying 103% complete.)

• I feel like this is a little obscure. I was thinking of splitting the logic into a private function with a decent name. Is it worth it? What would you name it?

  ' boxProgress.Width = a percentage of maximum box width
  Me.boxProgress.Width = (mCurrentProgress / mSteps) * MaxBoxWidth
  

Answer 1

Would it be cleaner to simply use a Public variable instead?

You're not really asking that question. I'm not really telling you that a property is always going to be cleaner than exposing a public field. ...Well look at that, I just did!

Point is, yes, any property could also be a public field. Heck, any public field might as well be a global variable instead. See where I'm going? Encapsulation is awesome, embrace it ;)

feel like this is a little obscure. I was thinking of splitting the logic into a private function with a decent name. Is it worth it?

' boxProgress.Width = a percentage of maximum box width
 Me.boxProgress.Width = (mCurrentProgress / mSteps) * MaxBoxWidth

All by itself, I see nothing obscure about this line; I'm not even sure I'd have that explanatory comment. But if you zoom out a little, and include a bit of context here...

Public Property Let CurrentProgress(lng As Long)
' Updating the CurrentProgress property updates the status of the Progress Bar
 mCurrentProgress = lng
 ' format #0 makes a 1 or 2 digit number without decimals
 If mMode = PBarMode_Percent Then
 Me.txtStatus = Format(Me.PercentComplete, "#0") & " % Complete"
 ElseIf mMode = PBarMode_Executing Then
 Me.txtStatus = "Executing..."
 End If
 ' boxProgress.Width = a percentage of maximum box width
 Me.boxProgress.Width = (mCurrentProgress / mSteps) * MaxBoxWidth
 Me.Repaint
 DoEvents
End Property

It's a property setter! I think a setter's job ends here:

 mCurrentProgress = lng

The rest of what happens in that Let-setter belongs in another method, perhaps something like... Refresh?

Public Property Let CurrentProgress(ByVal value As Long)
' Updating the CurrentProgress property updates the status of the Progress Bar
 mCurrentProgress = value
 Refresh
End Property

I wouldn't stop at Refresh - you have an Enum type, but you're If-ElseIfing through it. Use a Select Case here, Mr. Maintainer will like you when he needs to add a PBarMode_ManualCaption execution mode that displays an ad-hoc progress message ("Reticulating splines...") at each step; if there's a dedicated UpdateProgressMessage public method, it's even easier.

Your current implementation will [re-]set the Executing... text at every update in progress; I would make that message mode only update the progress message once instead.

And lng isn't a very good name for a value that represents progress. I like having a convention that properties themselves define the meaning of their paramater value with their name, so all my property [let-]setters take a value parameter. Maybe I'm just being lazy with naming though. But lng looks like a disemvoweled long... which looks like a Hungarian prefix without an actual variable name, a bit like str for a String.

So to recap on the CurrentProgress property, I would:

• Rename the parameter to value, and pass it ByVal instead of implicitly ByRef.
• Extract an UpdateProgressMessage method, to determine the status text.
• Extract an UpdateProgressValue method, to determine the length/value of the progress bar.

• Extract an Update or Refresh method, to call the other two extracted method:

  Private Sub Refresh()
   UpdateProgressMessage
   UpdateProgressValue
   Me.Repaint
   DoEvents
  End Sub
  

---

I cringe whenever I see a variable, field or property that's just called Mode. It's not you, it's just.. past experience working with someone's code where everything had a Mode, and Mode could be just about anything. So today I'm left with a severe reluctance to call something Mode. Maybe Mode is fine, but I'd prefer MessageDisplayMode, or MessageMode, or ProgressMessageMode... anything.

---

I don't like the m and e prefixes. The enum is easy to fix; the private fields though...

Private mMode As ePBarModeType
Private mCurrentProgress As Long
Private mSteps As Long

I know you've more than once commented on why I'm doing this, but I'd recommend shoving them into a Private Type - that way you can call them the same thing as the property they're backing:

Private Type TProgressBarMembers
 Mode As ePBarModeType
 CurrentProgress As Long
 Steps As Long
End Type

...and only have a single private field:

Private this As TProgressBarMembers

---

I don't like underscores either. The class itself is called Form_ProgressBar... VBA uses the underscore to denote event handlers and interface implementations; using it in an identifier feels wrong.

I'd rename the enum to something like this:

Public Enum ProgressBarDisplayMode
 DisplayPercent
 DisplayExecuting 
End Enum

Note that you don't need to specify any values when the first member's value is 0.

---

Overall I like the idea, but I find the names could use some fine-tuning. Other than that, you only have a few minor tweaks to make that code easier to extend, well done!

• RubberDuck: Thanks for the feedback! I was too close to it. The only thing to note is the class name. Access automagically prepends all forms with Form_. There's nothing I can do about that one. I'll be taking a lot of your advice though. Spot on about the setter doing more than just setting.

Answer 2

Adding the final version of the code here for anyone interested.

Option Explicit
' ***************************************************************
' Class: Form_ProgressBar 
' Popup progress bar 
' 
' Author: Christopher J. McClellan 
' http://christopherjmcclellan.wordpress.com/
' Significant input from Mat's Mug 
' http://codereview.stackexchange.com/users/23788/mats-mug
'
' Published under Creative Commons Attribution-Share Alike 
' http://creativecommons.org/licenses/by-sa/3.0/ 
'
' You are free to change, distribute, and pretty much do 
' whatever you like with the code, but you must give credit 
' to the original author and publish any derivative of this 
' code under the same license. 
' ***************************************************************
Public Enum ProgressBarDisplayMode
 PBarDisplayPercent
 PBarDisplayExecuting
End Enum
Private Type TProgressBarMembers
 Mode As ProgressBarDisplayMode
 CurrentProgress As Long
 Steps As Long
End Type
Private Const maxBoxWidth As Long = 7200
Private Const executingMessage As String = "Executing..."
Private this As TProgressBarMembers
Public Property Get PercentComplete() As Double
'read only
 PercentComplete = this.CurrentProgress / this.Steps * 100
End Property
Public Property Let Mode(newValue As ProgressBarDisplayMode)
 this.Mode = newValue
End Property
Public Property Get Mode() As ProgressBarDisplayMode
 Mode = this.Mode
End Property
Public Property Let CurrentProgress(newValue As Long)
 this.CurrentProgress = newValue
 ' keep the graphics in sync
 RepaintMe
End Property
Public Property Get CurrentProgress() As Long
 CurrentProgress = this.CurrentProgress
End Property
Property Let Steps(newValue As Long)
 this.Steps = newValue
End Property
Public Sub Init(Steps As Long, Mode As ProgressBarDisplayMode, Optional Caption As String = "Loading...")
 Me.Mode = Mode
 Me.Caption = Caption
 this.CurrentProgress = 0
 this.Steps = Steps
 Me.boxProgress.Width = 0
 Select Case Mode
 Case PBarDisplayExecuting
 Me.txtStatus = executingMessage
 Case Else
 Me.txtStatus = "Ready"
 End Select
 Me.Visible = True
End Sub
Private Sub RepaintMe()
 If Not this.Mode = PBarDisplayExecuting Then
 UpdateProgressMessage
 End If
 UpdateBoxWidth
 Me.Repaint
 DoEvents
End Sub
Private Sub UpdateProgressMessage()
 Select Case this.Mode
 Case PBarDisplayPercent
 ' format #0 makes a 1 or 2 digit number without decimals
 Me.txtStatus = Format(Me.PercentComplete, "#0") & " % Complete"
 Case PBarDisplayExecuting
 Me.txtStatus = executingMessage
 End Select
End Sub
Private Sub UpdateBoxWidth()
 Me.boxProgress.Width = (this.CurrentProgress / this.Steps) * maxBoxWidth
End Sub

Answer 3

Mat's Mug covered the issues with the form/class itself pretty well, but I'd like to mention a few things about exampleCall2(). This is boilerplate code that will be used anytime you call a long running ADODB.Command. It's absolutely worth extracting it into a method that takes in a command as an argument.

Private Sub ExecuteWithProgressBar(cmd as ADODB.Command)
 DoCmd.HourGlass True
 'must execute command async
 cmd.Execute , , adAsyncExecute
 Dim pbar As New Form_ProgressBar
 Dim i As Long
 pbar.init 10000, PBarMode_Executing, ""
 Do While cmd.STATE = adStateExecuting
 For i = 0 To 10000
 pbar.CurrentProgress = i
 Next i
 Loop
 Set pbar = Nothing
 DoCmd.Hourglass False
End Sub

Which still leaves a few issues to be dealt with.

1. You're messing with the UI, so you need to have an error handler that turns the hour glass mouse pointer back off. While you're at it, make sure to Set pbar = nothing to make sure it's removed from the screen.

2. This comment isn't very helpful.

   'must execute command async
   

   Of course we have to execute asynchronously! It's so obvious! Wait... Why do we have to execute async??? Try this instead.

   'must execute command async so 
   ' the code doesn't wait for cmd 
   ' to finish before showing pbar
   

3. Instead of hard coding the status message and number of steps to take, use some optional parameters with default values.

ExampleCall1 is boilerplate code too, but much harder to get rid of without delegates (which vba doesn't support). Any attempt to do so would probably cause more harm than good in my opinion.