First, I'd like to echo @Mat'sMug's distaste for your multi-linedDim statements. There's absolutely no reason to do thisat all having them prefixed withDim makes it completely obvious that I'm looking at a declaration block. It took me a good minute to realize that it wasn't a continuation ofSub declaration, because theDim literally disappears into a huge wall of code. You aren't saving any time typing it either - last time I checked,Dim and, _ were both 3 characters, and the former is alot easier to type on a qwerty keyboard.

The other answers hint around the performance, but I'll just come right out and say it. YourSub should really be named theAntiPerfWrap. First, every single call thatApplication.Run makes is late bound -even for Excel objects. It has to be, because likeCallByName, it is forced to callIDispactch.GetIDsOfNameson itself. This is always slower than an early bound call - period, end of discussion.

You always cache theActiveWorkbook andActiveSheet regardless of whether it is necessary or not. I would guess that roughly 1% of all the code I've written requires this. Same thing with.Calculation. Same thing with.EnableEvents. Do I write anotherPerfWrapWithEvents if I need that turned on? Or maybePerfWrapWithEventsAllowCalculation? None of this work is free, and it seems like a jumbled collection of "this will make your code go faster anecdotes" collected from SO comments. The cargo cult has found thier messiah in this wrapper.

DoEvents is not free. In fact it's the complete opposite. Ityields the processor to every other thread that needs to clear it's event queue. It also makes absolutely no sense to callDoEvents when you've explicitly disabled 95% of the functionality that would make the call useful.

Brass tacks time - let's see how much overhead this wrapper really adds. This is the called routine:

Public Sub UnderTest()    Dim i As Long    For i = 1 To 10        Debug.Print i    NextEnd Sub

And the baseline benchmark:

Public Sub BenchmarkOne()    Dim starting As Single    starting = Timer    Dim i As Long    For i = 1 To 100        Sheet1.UnderTest    Next    Debug.Print "Unwrapped: " & Timer - startingEnd Sub

Unwrapped: 0.8515625

Public Sub BenchmarkTwo()    Dim starting As Single    starting = Timer    Dim i As Long    For i = 1 To 100        PerfWrap "Sheet1.UnderTest"    Next    Debug.Print "Wrapped: " & Timer - startingEnd Sub

Wrapped: 6.492188

Ouch.

Conclusion - Burn it. Burn it with fire. The premise behind the code is that there is a common set of operations that you can do to "speed up" code. In fact, this isnever true. In reality, you are turning off Excel functionality that canslow down code. There's a big difference. If you approach performance issues by looking outside of code that you've written, you're starting out looking in the wrong direction. A catch-all "solution" for performance simply doesn't exist, so I wouldn't pretend to be providing it. The best way to look at it is to imagine that any code under performance review to be copied and pasted in place of this...

If ArgumentsToPass <> vbNullString Then    Application.Run SubNameToRun, ArgumentsToPassElse    Application.Run SubNameToRunEnd If

...then do a code review on the entire thing. Ask yourself, "In the context of the full Sub, what is the reason for every single line of code other than part above"? I'm guessing that in roughly 99.99% of cases, there won't be a good answer for all of them.

• 2
  \$\begingroup\$OUCH!!! Indeed! I couldn't imagine that it was THAT BAD... I knew that it wasn't good, but in fact it is quite an horror show!^^ I didn't know thatApplication.Run was so bad, and was just looking for something easier to use for the multiples UI actions. Anyway, thx for showing me how BAD it was! I'll get rid of this right now! ;)\$\endgroup\$

  R3uK

  – R3uK

  2017-03-20 12:27:57 +00:00

  CommentedMar 20, 2017 at 12:27
• 3
  \$\begingroup\$"The cargo cult has found thier messiah in this wrapper." I want to put this on my wall.\$\endgroup\$

  Brandon Barney

  – Brandon Barney

  2017-03-22 17:09:22 +00:00

  CommentedMar 22, 2017 at 17:09

You're inverting the caller/callee chain here. There are pro's and con's, but IMO more con's than pro's -Application.Run is a performance hit in the first place, and with the stringly-typed parameters you lose compile-time validation of your code... which I would doanything to try and preserve as much as possible.

Another downside is that it's not going to work on class methods, so you can't have an object that's responsible for interacting with a worksheet - may or may not be a problem, but utility code shouldn't be driving design decisions of its calling code that way.

This declaration is annoying:

Dim aWB As Workbook, _    ActiveSH As Worksheet, _    ScreenUpdateState As Boolean, _    StatusBarState As Boolean, _    CalcState As XlCalculation, _    EventsState As Boolean, _    DisplayPageBreakState As Boolean

You (or a maintainer) can't add a comment on any of those. This is probably going to break syntax highlighting in this post, but...

Dim aWB As Workbook, _ 'a comment here is illegal    ActiveSH As Worksheet, _    ScreenUpdateState As Boolean, _    StatusBarState As Boolean, _    CalcState As XlCalculation, _    EventsState As Boolean, _    DisplayPageBreakState As Boolean

Here's how the VBE sees it:

And if you manage to squeeze a legit comment in there, well you better hope you haveOption Explicit specified:

Dim aWB As Workbook, _    ActiveSH As Worksheet, _    ScreenUpdateState As Boolean 'oh, and this one is legal, but... _    StatusBarState As Boolean, _    CalcState As XlCalculation, _    EventsState As Boolean, _    DisplayPageBreakState As Boolean

I would simply avoid cramming multiple declarations in a single statement, there's no real reason to do that... and thereare real reasons to avoid it:

Variables should be declared close to their first use anyway, not in a wall of declarations at the top of the procedure.

FWIW withRubberduck (a COM add-in for the VBE that I [and other fellow VBA reviewers] is [are] working on), that's a one-click fix:

That instantly turns the statement into this:

Dim aWB As WorkbookDim ActiveSH As WorksheetDim ScreenUpdateState As BooleanDim StatusBarState As BooleanDim CalcState As XlCalculationDim EventsState As BooleanDim DisplayPageBreakState As Boolean

You don't need the theaWB local variable at all (a terrible name IMO), since you already haveActiveSH (which I'd probably rename tooriginalActiveSheet), and its.Parent member points to the originally active workbook.

It's not clear whySubNameToRun andArgumentsToPass are passedByRef (implicitly), whenDispStatusBar is explicitlyByVal - all parameters could very well beByVal here.

Actually - and given the 30 optional parameters ofApplication.Run that would lookmuch uglier, butArgumentsToPass would make a much friendlier API if it were aParamArray parameter; that way you could keep type safety for your arguments, which means you could pass arguments that can't be implicitly or explicitly converted to aString - e.g. aRange object.

Not being able to pass an object parameter is a serious showstopper limitation IMO.

I think it's a nice idea, but it's way too limited. A more flexible solution would be a simpleToggleWaitMode procedure that togglesScreenUpdating,DisplayStatusBar,Calculation,EnableEvents, allows specifying an optionalStatusBar message, and heck, that toggles theCursor to/fromxlWait as well.

You could even wrap that toggle behavior in a class, encapsulate the state (although that's aleaky abstraction, these things are all global really), and use theClass_Terminate handler to revert back to original state, that way the calling code couldn't even "forget" to toggle it back:

With WaitWorker.Init(statusText:="Please wait...", disableEvents:=True)    'do stuffEnd With

That way you don't need to doApplication.Run, so it works with class module callers, and you're not passing a stringly-typed list of parameters either, so the caller can still do everything it wants: you're notinterfering with the caller.

• 3
  \$\begingroup\$you is working on Rubberduck?\$\endgroup\$

  Vogel612

  – Vogel612

  2017-03-18 16:43:10 +00:00

  CommentedMar 18, 2017 at 16:43
• 4
  \$\begingroup\$@Vogel612 yes, I is =)\$\endgroup\$

  Mathieu Guindon

  – Mathieu Guindon

  2017-03-18 16:51:25 +00:00

  CommentedMar 18, 2017 at 16:51
• \$\begingroup\$Thx for the valuable inputs! I didn't though of the object parameters when I wrote the base that I reused more recently for this one (as I only use this on main program called by buttons), but indeed it is a deal-breaker for more general use (andParamArray is a pretty common coding method across languages) !ToggleWaitMode procedure could be interesting with a few Public variables, but I'm pretty intrigued by the class approach, and wonderingwhat do you mean by "leaky abstraction"?\$\endgroup\$

  R3uK

  – R3uK

  2017-03-20 12:12:44 +00:00

  CommentedMar 20, 2017 at 12:12
• \$\begingroup\$@Mat'sMug Out of curiosity, why should declarations be close to their use? I always thought it was opposite, but that may just be personal preference. I tend to declare whatever is easiest to use at the beginning, and then refactor for performance at the end. To this end, declaring them in a block at the top makes it much easier for me to see if I have gotten out of control and if I need to refactor.\$\endgroup\$

  Brandon Barney

  – Brandon Barney

  2017-03-22 17:14:16 +00:00

  CommentedMar 22, 2017 at 17:14
• 2
  \$\begingroup\$Oh, absolutely!! ...except if your boss is non-technical and the app is business-critical and you're just not authorized to check in non-functional changes that "don't bring any business value or fix any actual problem" =)\$\endgroup\$

  Mathieu Guindon

  – Mathieu Guindon

  2017-03-22 17:45:37 +00:00

  CommentedMar 22, 2017 at 17:45

I'm uncertain of the efficiencies in a single wrapper like this. In developing applications with multiple modules and classes, I'm often needing to disable screen updates and such but without extremely careful planning I used to always step on my own toes. So I developed two routines that allow me to disable and enable Excel application level updates and give me enough tracking debug.

Using the calls becomes as easy as sprinkling these calls around:

Sub SomeImportantWork()    DisableUpdates    '--- do stuff    OtherImportantWork    EnableUpdatesEnd SubSub OtherImportantWork()    DisableUpdates debugMsg:="enter OtherImportantWork"    '--- work more stuff    EnableUpdates debugMsg:="leave OtherImportantWork"End Sub

My idea has been to not worry about whether I've call disable/enable, but only to make sure those calls are paired around any code that requires that protection.

At the module level, I define a structure and a private variable to hold any previous state information:

Option ExplicitPrivate Type SavedState    screenUpdate As Boolean    calculationType As XlCalculation    eventsFlag As Boolean    callCounter As LongEnd TypePrivate previousState As SavedStatePrivate Const DEBUG_MODE = True

Then when I want to disable application updates, I call:

Public Sub DisableUpdates(Optional debugMsg As String = "", _                          Optional forceZero As Boolean = False)    With Application        '--- capture previous state if this is the first time        If forceZero Or (previousState.callCounter = 0) Then            previousState.screenUpdate = .ScreenUpdating            previousState.calculationType = .Calculation            previousState.eventsFlag = .EnableEvents            previousState.callCounter = 0        End If        '--- now turn it all off and count        .ScreenUpdating = False        .Calculation = xlCalculationManual        .EnableEvents = False        previousState.callCounter = previousState.callCounter + 1        '--- optional stuff        If DEBUG_MODE Then            Debug.Print "Updates disabled (" & previousState.callCounter & ")";            If Len(debugMsg) > 0 Then                Debug.Print debugMsg            Else                Debug.Print vbCrLf            End If        End If    End WithEnd Sub

And the mirroring method to re-enable is:

Public Sub EnableUpdates(Optional debugMsg As String = "", _                         Optional forceZero As Boolean = False)    With Application        '--- countdown!        If previousState.callCounter >= 1 Then            previousState.callCounter = previousState.callCounter - 1        Else            '--- shouldn't get here            Debug.Print "EnableUpdates ERROR: reached callCounter = 0"        End If        '--- only re-enable updates if the counter gets to zero        '    or we're forcing it        If forceZero Or (previousState.callCounter = 0) Then            .ScreenUpdating = previousState.screenUpdate            .Calculation = previousState.calculationType            .EnableEvents = previousState.eventsFlag        End If        '--- optional stuff        If DEBUG_MODE Then            Debug.Print "Updates enabled (" & previousState.callCounter & ")";            If Len(debugMsg) > 0 Then                Debug.Print debugMsg            Else                Debug.Print vbCrLf            End If        End If    End WithEnd Sub

As you can see, I have a private module flag to turn on debug messages (DEBUG_MODE) if I need them and turn them off for production. You can turn this into a compiler directive flag if you feel you need a (small) runtime performance boost.

ThecallCounter lets me keep track of my "updates call stack". This is especially usedful when I started calling some library routines I had and those calls got rather deep. It certainly helped to track it down.'

Also, I can go into theImmediate window at any time and typeEnableUpdates(forceZero=True) and get back to a known state manually.

• \$\begingroup\$Thx for your input, that what I thought about when reading the other answers. I like yourDEBUG_MODE which could be pretty handy indeed!\$\endgroup\$

  R3uK

  – R3uK

  2017-03-20 12:36:18 +00:00

  CommentedMar 20, 2017 at 12:36

• performance
• vba
• excel