About "Fast" Code
I take [mild] issue with "fast mode" and similar "optimize speed" wordings, because code that needs these switches toggled is very often code that is simply inefficient, and turning off calculations and screen repaints makes the macro complete faster, without making it any more efficient. Minimizing worksheet accesses (both reading and writing) normally makes the code run much faster, and when a macro involves a single worksheet read and a single worksheet write operation, toggling calculations and repaints off isn't going to change anything.
That said, there are times when such toggling is needed and/or beneficial. I'd go with HostApplicationState
or something similarly neutral as far as alluding to any performance gains goes - simply because when the code is efficient in the first place, then the effect of these toggles is very marginal.
Naming
FastMode
aside, NewStatus
is confusing and has no reason to not be just Status
: one of the advantages of using a Private Type
for the backing field of properties, is that you get to name each field exactly as per the property member it's backing. Naming it differently defeats that purpose and makes Property
members more confusing than necessary.
The NewValue
parameter name is appropriate for a Property Let
(or Set
) value parameter identifier name; Value
or RHS
(for right-hand-side) would also be appropriate.
State Corruption
The predeclared instance is stateful, which comes with a number of implications: the state is effectively global, and FastMode.Toggle
is working off that global state... but the Status
property being writable is making it very easy to misuse the class and essentially corrupt the state, which would be causing bugs downstream.
Because the Toggle
method assumes an enabled initial state, nesting instances would also corrupt the state.
Dim Test1 As FastMode '<~ this declaration is always legal
Set Test1 = New FastMode '<~ local instance has its own Status
FastMode.Toggle True, True, True '<~ Boolean arguments are bad for readability
Debug.Print FastMode '<~ default instance Status is True
Debug.Print Test1 '<~ local instance Status is still False
Test1 = True '<~ local instance write to Status is legal
FastMode = False '<~ manual toggle doesn't actually toggle the Application state
The Boolean
parameters are confusing and should be avoided altogether: it's hard to tell whether the Toggle
method is expecting True
or False
: the method isn't actually "toggling" the state so "true" isn't saying "yeah I want to toggle this one" but rather "make it true/false as I say", which is leaking the encapsulated global state into the abstraction.
So the first thing I would change would be to make Status
read-only by outright deleting the Property Let
member. The property being the class' default member isn't ideal, because it is parameterless. With the default/predeclared instance, that makes the meaning of a FastMode
token highly context-dependent. Parameterless default members are illegal in VB.NET where the Set
keyword is deprecated. Now, the Status
property is a Boolean
data type, so resolving the default member call isn't too hard: the member implicitly invoked is defined in the FastMode
class. When a class' default member returns another object type (which could have its own default member), resolving default member calls becomes a recursive operation, and we're looking at code that says one thing and does another.
This could be fixed in two ways:
- Query
This.FastMode
in the Toggle
method, making it work off the state of the current instance, i.e. each instance of the class gets its own state.
- Return
FastMode.Status
rather than This.Status
in the Property Get Status
member; conditionally return This.Status
when Me Is FastMode
: only the default instance actually holds any state.
Going with 2 is simpler, but then it makes instances of the FastMode
class rather deceptive, since the state is actually held in the default instance: because no instance is holding any state, it becomes very difficult to justify using a class over a standard module here, because OOP and global state don't really work well together.
Alternative
I would suggest a simpler API involving a With
block to eliminate the temporal coupling that forces the client code to invoke the Toggle
method - the calling code might look like this:
With FastMode.Toggle
'...do stuff...
End With
Or like this:
With FastMode.Toggle(AppState_Calculation Or AppState_ScreenUpdating)
'...do stuff...
End With
Side note: this Or
operator is bitwise-OR; a +
operator could be used instead, but using Or
in a bitwise context emphasizes the bitwise nature of the enum values.
The class' Initialize
handler would be responsible for encapsulating the initial application state at instance level, and the Terminate
handler would be responsible for restoring it... except for the default instance.
Because each instance is storing the application state as it is at the moment of initialization, instances can be safely nested; when an instance goes out of scope, application state gets restored to what it was when that instance was created. Guarding the mechanism against a stateful default instance seems wise here.
The Toggle
method becomes essentially a factory method that toggles the application state, optionally only affecting a subset. The result of this, is a more robust API that makes both With New FastMode
and With FastMode.Toggle
behave identically. Since a factory method is involved, we want to supply and implement an explicit interface that I'll call IAppState
here.
That IAppState
interface might look like this - note that all we need of this interface is for it to provide a way to access an instance's encapsulated initial state, so that's where we'll want to have a property per setting:
'@ModuleDescription "An object that encapsulates initial application state."
'@Interface
Option Explicit
Public Property Get InitialCalculation() As XlCalculation
End Property
Public Property Get InitialScreenUpdating() As Boolean
End Property
Public Property Get InitialDisplayAlerts() As Boolean
End Property
Next we'd have a HostApplicationState
class that implements this interface:
'@ModuleDescription "An object that toggles global application state as needed."
'@PredeclaredId
Option Explicit
Implements IAppState
Public Enum AppStateEnum
AppState_None = 0
'underlying values must be powers of 2:
AppState_Calculation = 1
AppState_ScreenUpdating = 2
AppState_DisplayAlerts = 4
'convenience member to include all properties:
AppState_All = AppState_Calculation Or _
AppState_ScreenUpdating Or _
AppState_DisplayAlerts
End Enum
Private Type TState
AppProperties As AppStateEnum
InitialCalculation As XlCalculation
InitialScreenUpdating As Boolean
InitialDisplayAlerts As Boolean
End Type
Private This As TState
'@Description "Disables the specified global application properties; properties will toggle back on when object goes out of scope."
Public Function Toggle(Optional ByVal AppProperties As AppStateEnum = AppState_All) As IAppState
Dim Result As HostApplicationState
Set Result = New HostApplicationState '<~ Initialize handler runs here
Result.AppProperties = AppProperties
If HasFlag(AppProperties, AppStateEnum.AppState_Calculation) Then
Application.Calculation = xlCalculationManual
End If
If HasFlag(AppProperties, AppStateEnum.AppState_ScreenUpdating) Then
Application.ScreenUpdating = False
End If
If HasFlag(AppProperties, AppStateEnum.AppState_DisplayAlerts) Then
Application.DisplayAlerts = False
End If
Set Toggle = Result
End Function
'@Description "Gets or sets the properties managed by this instance."
Public Property Get AppProperties() As AppStateEnum
AppProperties = This.AppProperties
End Property
Public Property Let AppProperties(ByVal Values As AppStateEnum)
This.AppProperties = Values
End Property
'@Description "A helper function performing bitwise logic for flag enums."
Private Function HasFlag(ByVal value As Long, ByVal flag As Long) As Boolean
HasFlag = (value And flag) = flag
End Function
Private Sub Class_Initialize()
'Default instance is stateless:
If Me Is HostApplicationState Then Exit Sub
'affect all managed properties by default (use .Toggle to narrow it down):
This.AppProperties = AppStateEnum.AppState_All
This.InitialCalculation = Application.Calculation
This.InitialScreenUpdating = Application.ScreenUpdating
This.InitialDisplayAlerts = Application.DisplayAlerts
End Sub
Private Sub Class_Terminate()
'Default instance is stateless:
If Me Is HostApplicationState Then Exit Sub
'only restore the properties that were toggled...
If HasFlag(This.AppProperties, AppStateEnum.AppState_Calculation) Then
Application.Calculation = This.InitialCalculation
End If
If HasFlag(This.AppProperties, AppStateEnum.AppState_ScreenUpdating) Then
Application.ScreenUpdating = This.InitialScreenUpdating
End If
If HasFlag(This.AppProperties, AppStateEnum.AppState_DisplayAlerts) Then
Application.DisplayAlerts = This.InitialDisplayAlerts
End If
End Sub
Private Property Get IAppState_InitialCalculation() As XlCalculation
IAppState_InitialCalculation = This.InitialCalculation
End Property
Private Property Get IAppState_InitialScreenUpdating() As Boolean
IAppState_InitialScreenUpdating = This.InitialScreenUpdating
End Property
Private Property Get IAppState_InitialDisplayAlerts() As Boolean
IAppState_InitialDisplayAlerts = This.InitialDisplayAlerts
End Property
Implemented like this, state is no longer global, and each instance encapsulates its own Initial[ApplicationProperty]
values to use for restoring the Application
state to exactly what it was before the object came into existence.
Now the API is looking like this:
With HostApplicationState.Toggle '<~ all managed properties get disabled
'note: the only available members are the .Initial[ApplicationProperty] getters
'...do stuff...
End With '<~ all managed properties get toggled back on
That is functionally equivalent to this - note the Toggle
member call; including it in the Initialize
handler would remove the need to have it in the client code here, but then it would also make it needlessly invoked twice when using the preferred factory method, and we don't want a side-effecting Initialize
handler anyway so if client code decides to New
up the class, Toggle
needs to be explicitly invoked:
With New HostApplicationState
.Toggle '<~ all managed properties get disabled
'note: .Toggle and .AppProperties members are accessible, but not the .Initial[ApplicationProperty] getters.
'...do stuff...
End With '<~ all managed properties get toggled back on
While it "works", it isn't leveraging the class' default instance, which makes the VB_PredeclaredId
attribute useless. Still, it would probably work fine without breaking anything. Obviously it can be abused by changing the AppProperties
value before the object goes out of scope, but then such wrong code would look wrong, and that's perfect (and guard clauses can actively prevent such misuse by throwing an error in the Property Let
scope).
And if do stuff
involves invoking a procedure that specifically needs alerts turned off, then that macro can do this:
With HostApplicationState.Toggle(AppState_DisplayAlerts)
'note: Application.DisplayAlerts state before the With block is irrelevant
'...do stuff
End With
And when that instance goes out of scope (i.e. when the With
block is exited - note that this covers jumping out of the With
block on error); if that instance is nested and the outer instance has already toggled DisplayAlerts
, then this inner instance is redundant because its InitialDisplayAlerts
is going to be False
- by encapsulating the initial state, it's making fewer assumptions.
Perhaps Manage
would be a better name than Toggle
here:
With HostApplicationState.Manage(AppState_Calculation Or AppState_ScreenUpdating)
'...do stuff...
With HostApplicationState.Manage(AppState_DisplayAlerts)
'...do more stuff... (calc and repaints are still disabled)
End With '<~ alerts are back on here
'...do stuff...
End With '<~ calc and repaints are re-enabled here (assuming "enabled" initial state)
Of course the biggest caveat with such a class with a side-effecting Terminate
handler is that you want to limit its lifetime as much as possible (With
blocks are perfect for this!), so the ultimate misuse of that class would be to have a module-scope variable of that type.
Lastly, note that there is no need for a Boolean
value to track whether state is enabled or not: that "active" state is made explicit by the caller using a With
block to withold the object instance. In other words if the object is alive, it is also active: the caller is in charge of the object's lifetime, and therefore of exactly when the application state gets toggled off and/or back on.
'@Folder()
. Smiles at another RubberDuck user! \$\endgroup\$