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
3 Answers 3
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 $N13ドル
some 2,000 times with a value that's in "$B$" & i
, and then I guess $U12ドル
, $V12ドル
and $W12ドル
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 $N13ドル
changes, then you can force calculation like this:
wsInput.Range("$U12ドル").Calculate
wsInput.Range("$V12ドル").Calculate
wsInput.Range("$W12ドル").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, $AA4ドル:$AC2004ドル
) 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!$AA4ドル:$AC2004ドル
to Repository!$C4ドル:$E2004ドル
.. 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 =$X12ドル*$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
.
Two short things that even may be unneeded if yo u excluded them.
Use Option Explicit
For one it helps immensely when searching for errors with spelling. Been there done that. It's gruesome.
Secondly it helps you with writing code that's more similar to "real" programming languages. Nothing against vba, but I much prefer languages where you need to declare your variables.
Use an error handler.
Everytime you turn off screen updating you get into the dangerous zone of "breaking" the application in case somethin goes wrong.
Instead do something along the lines of:
Option Explicit
On Error Goto ErrorHandler
' a whole lot of code
:ErrorHandler
Application.ScreenUpdating = true
MsgBox "An error has occurred"
'Whatever else you need to do and probably something like
Exit Sub
I'm not sure this will work for you, but consider replacing or duplicating the formulas in your "12" cells with an Evaluate
call. It's a little tricky to avoid runtime errors, so I suggest reading this. It might look something like this.
'wsRepository.Range("E" & i).Value = wsInput.Range("U12").Value
wsRepository.Range("E" & i).Value = Evaluate("SUM(A1:A10)")
Of course, using the formula in U12
. Your mileage may vary, but this should let you set calculation to manual (I think). I would prefer the method Mat's Mug described though. This is just another option to try.
Some other notes
i
is typically used as a loop counter, butrow
would be more meaningful.4
and2004
are mysterious hardcoded numbers. What a lot of programmers refer to as magic numbers. It would good for readability/maintainability to replace them withstartRow
andlastRow
constants.- Great comments in my opinion. They're short and clear. Not too much, not too little.
- I'm not sure why you activate
wsInput
at the end, but you do a great job of avoiding it elsewhere, make sure you're not needlessly activating the sheet there.
-
\$\begingroup\$ I wouldn't expect
Evaluate
to speed things up though. \$\endgroup\$Mathieu Guindon– Mathieu Guindon2014年08月18日 01:09:23 +00:00Commented Aug 18, 2014 at 1:09 -
2\$\begingroup\$ No. It wouldn't, but it may allow OP to use manual calculation @Mat'sMug and that would speed it up. (Assuming there are many other cells that wouldn't need to be recalculated). It's a stretch, but may be worth a shot. \$\endgroup\$RubberDuck– RubberDuck2014年08月18日 01:13:34 +00:00Commented Aug 18, 2014 at 1:13
U12
, etc.? Can you share them? It would be easier to help if you can. \$\endgroup\$