1
\$\begingroup\$

Using a technique described in this StackOverflow answer by @David Murdoch I created a custom class with methods to speed up Excel (Activate turns off screen updates and set the calculation mode to manual) and to restore it to normal settings (Shutdown). When the object reaches the end of its life and Class_Terminate is triggered it runs the Shutdown procedure.

The purpose of all this is to avoid problems that occur if (when) I forget to reset these settings at the end of a script or if my code (contains a bug that) skips the section resetting these settings.

I am looking for a broad critique of the nit-picky variety. In other words, how you would code this.

This code intentionally ignores the initial state of the settings and always turns on screen updating and sets the calculation mode to automatic when the object is destroyed.

The following is in a class module called ExcelTurboBooster.

Option Explicit
'
'  Use the code below to turn off screen updates and automatic
'   calculations.  Screen updates and automatic calculations will be
'   restored when the calling procedure terminates.
'
' EXAMPLE USAGE
' ======================================================================
'   Dim objExcelTurboBooster As New ExcelTurboBooster
'   objExcelTurboBooster.Activate   ' Activates booster
'   objExcelTurboBooster.Shutdown   ' Use to deactivate before procedure
'                                   '  terminates
' ======================================================================
'
'
Public Sub Activate()
    Application.Calculation = xlCalculationManual
    Application.ScreenUpdating = False
End Sub

Public Sub Shutdown()
    On Error Resume Next
    Application.Calculation = xlCalculationAutomatic
    Application.ScreenUpdating = True
End Sub

Private Sub Class_Terminate()
    Shutdown
End Sub
\$\endgroup\$
1
\$\begingroup\$

I think you have a potential usage issue with wrapping these methods in a class. While it's convenient that the Class_Terminate method will automatically re-enable updating, it makes keeping track of the usage of such an object the responsibility of the caller (or calling module or of the entire project). Where you can easily get into trouble is when you start making calls to other objects or functions that might need to make the same disable/enable actions for speed. This is especially apparent if you create a personal library of routines that you can reuse.

My preferred approach to this was to create a standalone module that I can easily import into any VBA project and use immediately. The public methods are DisableUpdating and EnableUpdating. In every day use I can now create:

Sub MainProgram()
    DisableUpdating
    ...
    LibrarySub1 <some vars>
    ...
    EnableUpdating
End Sub

Sub LibrarySub1(<some vars>)
    DisableUpdating
    ...
    EnableUpdating
End Sub

But you can see the potential problem in nesting these calls.

My solution in the standalone module is:

Option Explicit

Private updateNestCount As Long
Private calculationState As XlCalculation

Public Sub DisableUpdating(Optional msg As String = vbNullString)
    If updateNestCount = 0 Then
        Application.ScreenUpdating = False
        '--- save the current calculation state for later restoration
        calculationState = Application.Calculation
        Application.Calculation = xlCalculationManual
    End If
    updateNestCount = updateNestCount + 1

    '--- print a debug message if the caller wants one
    If Len(msg) > 0 Then
        Debug.Print "DisableUpdating(" & updateNestCount & "): " & msg
    End If
End Sub

Public Sub EnableUpdating(Optional msg As String = vbNullString)
    If updateNestCount = 1 Then
        Application.ScreenUpdating = False
        '--- restore the calculation back to its original state
        Application.Calculation = calculationState
    End If
    updateNestCount = updateNestCount - 1

    '--- print a debug message if the caller wants one
    If Len(msg) > 0 Then
        Debug.Print "EnableUpdating (" & updateNestCount & "): " & msg
    End If
End Sub

Now I can liberally sprinkle these Disable/Enable calls in my libraries and other routines and the private variable takes care counting how deeply nested it might be. I've run into the need to track some debug on occasion, so the optional msg comes in handy.

I think your class/object approach leaves too much of the burden on the calling program to keep track of the object lifetime and you might find this a simpler approach. Just make sure to always pair the calls in a single routine AND if you have an error handler, that the enable method is accessed within the error handling.

\$\endgroup\$
  • \$\begingroup\$ I like your approach! It can avoid unnecessary enabling and disabling of features. That's particularly useful when a full workbook recalculation takes a long time since setting the calculation mode to automatic triggers a recalculation. \$\endgroup\$ – ChrisB Apr 2 '18 at 21:47
2
\$\begingroup\$

I would point out that the naming convention is inconsistent -- we have Activate and a Shutdown --- they don't say the same thing to me. I'd expect corresponding actions to have names like Activate -> Deactivate, Enable -> Disable, Startup -> Shutdown. Given the nature of the class, Enable/Disable pair seems the best suited for what your class is going to do, since it's basically toggling an application level state.

Your Shutdown has an On Error Resume Next but says nothing if it was successful. I would at least expect the procedure to exit with something like ShutDown = (Err.Number = 0) so that the calling client can at least know there was a clean exit or not. However, I'm more inclined to encourage to rethink whether you really want a Resume Next there, because you are dealing with altering the application's state, so it is crucial to know that the alterations were successful so that the clients can proceed with the assumption that they are in the correct state.

The other thing that occurs to me is that you probably don't actually need the methods at all. You only need the Class_Initialize and Class_Terminate. Your calling code then becomes...

Set booster = New ExcelTurboBooster
'Do lot of complicated stuff....
Set booster = Nothing 

Look, ma, no methods! In fact, technically you only need the first line. The last line is superficial because when your procedure that instantiate the object exits (even if it exited badly), the instance will go poof and the application state should be reset. The biggest downside of that method, though, is that error handling becomes somehow strange inside the Initialize/Terminate events, so it could be difficult to communicate to your clients there has been a problem.

The downside with the no-method approach is that it totally can't go into a public field of any standard module or maybe long-lived classes either because then they will persist too much longer. One'd have to take discipline to use it only as a local variable inside procedure so that it will be destroyed at the end of the procedure. Otherwise, you've built up a new layer of complication for nothing.

\$\endgroup\$
1
\$\begingroup\$

It's dangerous to not save the state of Application.Calculation and Application.ScreenUpdating.

Also, what's the purpose of the On Error Resume Next? There's no way for those two calls to fail, and the error handling gets turned back on when Shutdown exits.

\$\endgroup\$

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.