HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Inefficient (slow) loop with calculations in worksheet

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withloopslowinefficientcalculationsworksheet

Problem

I am looking for best practice help. Slow loops seem to be a recurring problem for me and I'd like to learn a better way. The code itself works as it should, except it is far too slow.

The problem is the worksheet needs to calculate after each B & i value is dropped into "N13" so that "U12", "V12", and "W12" update before being deposited into wsRepository. If I turn Calculation on Manual then my values are no good because they are contingent upon the other worksheet formulas updating (calculating). I think I can copy my worksheets "off-screen", calculate, and then paste my values back "on-screen", but I am not sure how to do this. I've used variants in the paste to do similar things, but I not comfortable with them. There may even be more efficient ways of achieving my desired result that I am unaware of.

Application.ScreenUpdating = False

Dim wsRepository As Worksheet
Dim wsInput As Worksheet
Dim i As Integer

Set wsRepository = ThisWorkbook.Sheets("Repository")
Set wsInput = ThisWorkbook.Sheets("Input")

For i = 4 To 2004

           'add investment amount
           wsInput.Range("N13").Value = wsRepository.Range("B" & i).Value

           'copy back amounts
           wsRepository.Range("E" & i).Value = wsInput.Range("U12").Value
           wsRepository.Range("C" & i).Value = wsInput.Range("V12").Value
           wsRepository.Range("D" & i).Value = wsInput.Range("W12").Value

    Next i

wsInput.Activate

Solution

Application.ScreenUpdating = False


I find it scary that the corresponding = True is nowhere in your code, for reasons already mentioned. Whenever I turn off screen updating, I find it's good UX to also specify a status bar message, and change the mouse cursor to a hourglass. Something along these lines:

Public Sub ToggleWaitMode(Optional ByVal waitMode As Boolean = False)
    Application.ScreenUpdating = waitMode
    Application.Calculation = IIf(waitMode, xlCalculationManual, xlCalculationAutomatic)
    Application.StatusBar = IIf(waitMode, "Please wait...", vbNullString)
    Application.Cursor = IIf(waitMode, xlWait, xlDefault)
End Sub


Which makes your procedure stub look like this:

Option Explicit

Public Sub DoSomething()
    On Error GoTo ErrHandler
    ToggleWaitMode True

    'do that thing

CleanExit:
    If Not Application.ScreenUpdating Then ToggleWaitMode
    Exit Sub
ErrHandler:
    ToggleWaitMode
    ' handle errors here
    Resume CleanExit
End Sub



If I turn Calculation on Manual then my values are no good because...

If I understand properly, you need to update $N$13 some 2,000 times with a value that's in "$B$" & i, and then I guess $U$12, $V$12 and $W$12 need to be recalculated accordingly.

You haven't shown us what these cells contain and what cells their formula is referring to, but if they're the only cells that need to be recalculated when $N$13 changes, then you can force calculation like this:

wsInput.Range("$U$12").Calculate
wsInput.Range("$V$12").Calculate
wsInput.Range("$W$12").Calculate


But that might not speed up anything. You're pretty much stuck, since you need to recalculate these three cells before you can do anything, and you need to do that 2,000 times.

I think you're somewhat misusing VBA here, it looks like you could use 3 hidden columns (say, $AA$4:$AC$2004) and use Excel formulas to automatically calculate the would-be "U", "V" and "W" values for each row; the VBA macro could then just copy values from Input!$AA$4:$AC$2004 to Repository!$C$4:$E$2004.. if a macro is even needed for that.

I would suggest naming the ranges/cells in row 12 - anytime you have a specific cell with a specific meaning, it's always better for the VBA code to refer to the meaning rather than the cells' addresses.

I have no clue what these cells mean, but picture this:

Dim interestRate As Double
interestRate = wsInput.Range("InterestRate").Value


This extra abstraction level somewhat decouples the VBA code from the worksheet structure, which allows you to modify [at least parts of] the worksheet without having to modify the VBA code - for example you could insert another row and now InterestRate is read in row 13 instead of 12, and the VBA code couldn't care less.

You can define names in the [Formulas] Ribbon tab, under the [Defined Names] section. Or you can just select the cell and type its name in the address/names dropdown, just left of the formula bar.

This also has the advantage of making your Excel formulas more readable: instead of =$X$12$N42 a formula can now look like =InterestRate$N42

One last thing, I know it's common to call a worksheet variable like wsInput, but I find it sounds backwards and looks Hungarian. I'd call it inputSheet instead; wsRepository would be repositorySheet. Also it wouldn't hurt to rename i for row.

Code Snippets

Application.ScreenUpdating = False
Public Sub ToggleWaitMode(Optional ByVal waitMode As Boolean = False)
    Application.ScreenUpdating = waitMode
    Application.Calculation = IIf(waitMode, xlCalculationManual, xlCalculationAutomatic)
    Application.StatusBar = IIf(waitMode, "Please wait...", vbNullString)
    Application.Cursor = IIf(waitMode, xlWait, xlDefault)
End Sub
Option Explicit

Public Sub DoSomething()
    On Error GoTo ErrHandler
    ToggleWaitMode True

    'do that thing

CleanExit:
    If Not Application.ScreenUpdating Then ToggleWaitMode
    Exit Sub
ErrHandler:
    ToggleWaitMode
    ' handle errors here
    Resume CleanExit
End Sub
wsInput.Range("$U$12").Calculate
wsInput.Range("$V$12").Calculate
wsInput.Range("$W$12").Calculate
Dim interestRate As Double
interestRate = wsInput.Range("InterestRate").Value

Context

StackExchange Code Review Q#60323, answer score: 4

Revisions (0)

No revisions yet.