I'm building a template [Process-Tracker] spreadsheet.
The idea is that, for any process where we can define what steps should occur in what order (and preferably, how far apart), there will be a spreadsheet with buttons for:
- Adding a new [Item] to be tracked
- Updating the List of [Items] and their progress
- and a list of [Next Steps] ordered by [Due date].
I have written the [template] code for the Buttons and UserForm. The idea is that, when creating an actual process, I simply add specific validation for the expected variables/names and make sure it still outputs in a consistent format, and everything else will run just fine.
Here is my Form and My Code:
![enter image description here
[Worksheet] Add Item Button
Public Sub Button_AddItem_Click()
CallFormInput
End Sub
Form Input Sub
Public Sub CallFormInput()
UserFormButtonWasPressed = False
UF_New_Process_Item.Show
If Not UserFormButtonWasPressed Then PrintErrorMessage "Please only exit the Form via one of the buttons provided", showMessageBox:=True, endExecution:=True
End Sub
UserForm Code
Option Explicit
Option Compare Text
Private values As Collection
Private Sub UF_Exit_Form_Click()
TestFormSyntax
UserFormButtonWasPressed = True
UF_New_Process_Item.Hide
Exit Sub
End Sub
Private Sub UF_Add_Item_Click()
Dim values As Collection
Dim inputsWithFailedValidation As Variant
inputsWithFailedValidation = Array()
Dim validationMessage As String, passedValidation As Boolean
Set values = New Collection
UserFormButtonWasPressed = True
ValidateAndAssignValues values, validationMessage, passedValidation, inputsWithFailedValidation
If passedValidation Then
AddItem values
Else
HandleFailedValidation validationMessage, passedValidation, inputsWithFailedValidation
End If
End Sub
Private Sub UF_Add_Item_Recurring_Click()
Dim values As Collection
Dim inputsWithFailedValidation As Variant
inputsWithFailedValidation = Array()
Dim validationMessage As String, passedValidation As Boolean
UserFormButtonWasPressed = True
ValidateAndAssignValues values, validationMessage, passedValidation, inputsWithFailedValidation
If passedValidation Then
AddItem values
Button_AddItem_Click '/ Starts again as if clicked the "Add Item" Button on the worksheet
Else
HandleFailedValidation validationMessage, passedValidation, inputsWithFailedValidation
End If
End Sub
Private Sub ValidateAndAssignValues(ByRef values As Collection, ByRef validationMessage As Variant, ByRef passedValidation As Boolean, ByRef inputsWithFailedValidation As Variant)
AssignValues values
ValidateValues values, validationMessage, passedValidation, inputsWithFailedValidation
End Sub
Private Sub AssignValues(ByRef values As Collection)
'/ Pass all values as text, formatting/validation will be handled later
Dim i As Long
Dim controlType As String
Dim uf_Control As Control
'/ item = control item text, key = control item name
Set values = New Collection
With Me.Controls
For i = 0 To .Count - 1
Set uf_Control = .Item(i)
controlType = TypeName(uf_Control)
If controlType = "textbox" Then
values.Add uf_Control.Text, uf_Control.name
End If
Next i
End With
End Sub
Private Sub ValidateValues(ByRef values As Collection, ByRef validationMessage As Variant, ByRef passedValidation As Boolean, ByRef inputsWithFailedValidation As Variant)
'/ Convert to correct varType and validate - Names, Emails, Phone Numbers, Dates etc.
'/ If failed validation, add to validation Message, add me.controls.item(key) to inputsWithFailedValidation
'/ Set passed Validation True/False
passedValidation = True
End Sub
Private Sub AddItem(ByRef values As Collection)
UF_New_Process_Item.Hide
Dim inputValues As Variant
inputValues = Array()
ReorderValuesCollection values, inputValues '/ Explicitly re-build values in a set order in a new array
CreateNewProcessItem inputValues
End Sub
Private Sub HandleFailedValidation(ByRef validationMessage As Variant, ByRef passedValidation As Boolean, ByRef inputsWithFailedValidation As Variant)
Dim uf_TextBox As TextBox
PrintErrorMessage validationMessage, showMessageBox:=True, stopExecution:=False
If ArrayIsAllocated(inputsWithFailedValidation) Then
AssignArrayBounds inputsWithFailedValidation, LB1, UB1
For ix = LB1 To UB1
Set uf_TextBox = inputsWithFailedValidation(ix)
uf_TextBox.Text = ""
Next ix
Else
PrintErrorMessage "Validation failed but no objects specified", showMessageBox:=True, stopExecution:=True
End If
End Sub
Private Sub TestFormSyntax()
End Sub
As well as general "Could I have written my code better" Feedback, I am particularly interested in how my code works as a template.
Could it be refactored/generalised even further?
Could I structure it to be even easier to modify?
Have I used a good (generic) naming convention re: distinguishing UserForm objects/subs from the rest of the program?
1 Answer 1
You have added logic to an event of a button, which is not the best choice. You should extract most of the logic to at least separate methods or much better to a separate class.
First let us take a look at this
Private Sub ValidateValues(ByRef values As Collection, ByRef validationMessage As Variant, ByRef passedValidation As Boolean, ByRef inputsWithFailedValidation As Variant) '/ Convert to correct varType and validate - Names, Emails, Phone Numbers, Dates etc. '/ If failed validation, add to validation Message, add me.controls.item(key) to inputsWithFailedValidation '/ Set passed Validation True/False passedValidation = True End Sub
I assume that you will implement proper validation instructions later on, but I wanted to point out that having a Sub
instead of a Function
which is taking a ByRef Boolean
which is later queried isn't the way to go.
So let us change this to a function to get rid of that parameter.
Private Function ValidateValues(ByRef values As Collection, ByRef validationMessage As Variant, ByRef inputsWithFailedValidation As Variant) As Boolean
ValidateValues = true
End Function
The Sub AssignValues
should be changed to a Function GetValues() As Collection
because you have a Sub
with only a ByRef
parameter. As it seems that the form won't have more controls than 255 we can also change the Dim i As Long
to Dim i As Integer
.
Private Function GetValues() As Collection
'/ Pass all values as text, formatting/validation will be handled later
Dim i As Integer
Dim controlType As String
Dim uf_Control As Control
Dim values As Collection
'/ item = control item text, key = control item name
Set values = New Collection
For i = 0 To Me.Controls.Count - 1
Set uf_Control = Me.Controls.Item(i)
controlType = TypeName(uf_Control)
If controlType = "textbox" Then
values.Add uf_Control.Text, uf_Control.name
End If
Next i
Set GetValues = values
End Function
Now let's switch to UF_Add_Item_Click()
which is having almost identical code like the UF_Add_Item_Recurring_Click()
method. This code duplication should be extracted to a separate method.
So let us create a method ProcessItem
Private Function ProcessItem() As Boolean
ProcessItem = True
Dim values As Collection
Dim inputsWithFailedValidation As Variant
inputsWithFailedValidation = Array()
Dim validationMessage As String
Dim passedValidation As Boolean
UserFormButtonWasPressed = True
Set values = GetValues()
If Not ValidateValues(values, validationMessage, inputsWithFailedValidation) Then
HandleFailedValidation validationMessage, passedValidation, inputsWithFailedValidation
ProcessItem = False
Else
AddItem values
End If
End Sub
I have changed the style the local variables are defined, because having multiple variables defined on the same line should be avoided for readability.
I have added the call to AssignValues
which helps to get rid of the ValidateAndAssignValues
method.
Now the former UF_Add_Item_Click()
and UF_Add_Item_Recurring_Click()
method will look like so
Private Sub UF_Add_Item_Click()
ProcessItem
End Sub
Private Sub UF_Add_Item_Recurring_Click()
If ProcessItem() Then
Button_AddItem_Click '/ Starts again as if clicked the "Add Item" Button on the worksheet
End If
End Sub
I can't tell much about the AddItem
method because there are calls to methods which aren't in the posted code.
-
\$\begingroup\$ That would be because (at the time) AddItem hadn't been coded yet :) \$\endgroup\$Kaz– Kaz2015年10月13日 09:11:48 +00:00Commented Oct 13, 2015 at 9:11
"Please only exit the Form via one of the buttons provided"
- you could just hide theX
on the form \$\endgroup\$