My VBA code currently loops for 5,000 iterations and takes about 25 minutes to collect the output data in the Output excel sheet. Is there a way to reduce the time taken for instance by improving the copying and pasting of values, collecting the output data in some kind of array etc.? Given that I will be looping it for up to 10,000 iterations when I obtain more data
Note: The calculations are excel formulas within the Calcs excel sheet, as this is a cash flow schedule with irregular cash flows and discount rates, hence I am unable to automate the cash flow calculations via VBA.
Sub x()
Dim r As Long
Dim lastrow As Long
Application.ScreenUpdating = False
Application.CutCopyMode = False
With Worksheets("MCInput")
lastrow = .Cells(.Rows.Count, "B").End(xlUp).Row
MsgBox lastrow - 1 & " Trials"
End With
With Worksheets("Calcs")
For r = 0 To (lastrow - 2)
.Range("CASHINPUT").Value = Worksheets("MCInput").Range("TCASHRTN").Offset(r).Value
.Range("EQINPUT").Value = Worksheets("MCInput").Range("TEQRTN").Offset(r).Value
.Range("FIINPUT").Value = Worksheets("MCInput").Range("TFIRTN").Offset(r).Value
.Range("Results").Calculate
.Range("Results").Copy
Worksheets("Output").Range("A1").Offset(1 + r, 1).PasteSpecial xlPasteValues
Next r
End With
Application.ScreenUpdating = True
Application.CutCopyMode = True
End Sub
2 Answers 2
Use descriptive variable names. r
as a variable name tells you nothing and makes your code harder to read. rowOffset
tells you what it's used for. This goes a long way to make your code readable.
Declare your variables just before you use them. You only have 2 currently but its easy for variables to increase. Instead of having a wall of declarations at the top of a Function/Sub, like below:
dim r as long
Dim i as long
dim p as range
dim sv as range
Have them declared just before they get used.
dim lastRow as long
lastRow = Worksheets("MCInput").Cells(Rows.Count,"B").End(xlUp).Row
dim rowOffset as long
For rowOffset = 0 to (lastRow - 2)
`Code that does what you want
Next
This ultimately aids you in refactoring (restructuring) your code when you need to. It also becomes easier to tell when a variable is no longer needed. No code using the variable just after it's declared? Delete it. With a wall of declarations you have to laboriously check for usage. There are tools like Rubberduck, which I'm a contributor to, than can tell you if a variable is used.
You're using a string literal populated with a worksheets name to access the worksheet through the hidden Global object's Worksheets property IE Global.Worksheets("Calcs")
. This returns an Object
variable which is late-bound and doesn't give you intellisense when you use .
(period) after the collection. It's preferred to use the Worksheet.CodeName. You can see this in the Project Explorer window. The CodeName of the worksheet is Sheet1
while the Name (What is shown in the tab) is Calcs
.
Sheet1 isn't a helpful name, this ties into using descriptive names above, so we'll change it by double clicking on it in the Project Explorer to display the code behind for the worksheet, then pressing F4 to display the Properties Window (also found under the menu at the top View>Properties. At the top where it says (Name) change it to something more descriptive.
Now instead of using Worksheets("Calcs")
use CalculationSheet
to access the worksheet object. The same thing can be done to. Two added benefits:
- You're no longer dependent on the literal string "Calcs" which stops working if the worksheet is renamed.
- You get intellisense. Type
CalculationSheet.
(note the period) to show a list of accessible members.
There's more that can and probably should be done with your code. Things like worksheet properties would help to clean up and shorten the code which then leads to the realization you'd benefit from creating a proxy for accessing your data. I was nerd-sniped by this already and that can be for another review.
Thanks, guys. I got the answer on another forum. I made it store the calculated data outputs in an array before performing a one-time dump into another excel sheet. This halved my time taken to run the loops.
End Sub
but no(Public | Private) Sub FooBar()
to start it. I'm assuming it goes above your variable declarations, but that could be incorrect. \$\endgroup\$Results
range calculating and why does it need to be calculated by Excel, as opposed to being computed by VBA code? If that calculation doesn't need to happen on the worksheet, then you can collect the inputs, perform all calculations, and dump all results at once, with 1 single worksheet read and 1 single worksheet write. \$\endgroup\$x
though? You see naming is one of the hardest things in programming: giving a meaningful name to a procedure that does too many things can even be impossible! Typically, procedure names start with a verb, and describe the purpose or side effects of the procedure... anyway it would be nice to have the formula for theResults
range, and enough context to be able to determine whether a formula whose result is copied and then pasted, is the most efficient way to go about it. \$\endgroup\$