I posted on the Stack Overflow. They recommended me to post on here
So the whole scope of this Excel file is to copy and paste from other 27 external files to the current Excel file one by one. to show what I mean, following are the code examples and a stimulated capture picture.enter image description here
Macros(line # including space line):
1. Importing Sub
In my file, I have 27 subs like this. It's longer than this example. My real macro has 179 lines as the total. In this example, it only has 51 lines.
The only thing will be changed is the row numbers as the word row in VBA code in line 6.
Sub Import_NJ()
Dim Row As Integer, PathFileOpen As String, NameFileOpen As String,
TypeFileOpen As String, FullFileName As String, TabCopy As String, ModelFileName As String
Let Row = Worksheets("Control_Table").Cells("2", "D").Value
Let PathFileOpen = Worksheets("Control_Table").Cells(Row, "A").Text
Let NameFileOpen = Worksheets("Control_Table").Cells(Row, "B").Text
Let TypeFileOpen = Worksheets("Control_Table").Cells(Row, "C").Text
Let FullFileName = PathFileOpen & "\" & NameFileOpen & TypeFileOpen
Let TabCopy = Worksheets("Control_Table").Cells(Row, "J").Text
Let ModelFileName = Worksheets("Control_Table").Cells("10", "B").Text
Application.AskToUpdateLinks = False
Application.DisplayAlerts = False
Application.Calculation = xlCalculationManual
Workbooks.Open FileName:=FullFileName, UpdateLinks:=0
'Copy Income Statement
Workbooks(NameFileOpen).Worksheets("Total_Reports").Cells("9", "C").Resize(5, 120).Copy 'Revenues
Workbooks(ModelFileName).Worksheets(TabCopy).Cells("4", "AW").Resize(5, 120).PasteSpecial xlPasteValues
Workbooks(NameFileOpen).Worksheets("Total_Reports").Cells("18", "C").Resize(4, 120).Copy 'Prod Costs
Workbooks(ModelFileName).Worksheets(TabCopy).Cells("11", "AW").Resize(4, 120).PasteSpecial xlPasteValues
Workbooks(NameFileOpen).Worksheets("Total_Reports").Cells("25", "C").Resize(26, 120).Copy 'Employee Related thru maintenance
Workbooks(ModelFileName).Worksheets(TabCopy).Cells("17", "AW").Resize(26, 120).PasteSpecial xlPasteValues
Workbooks(NameFileOpen).Worksheets("Total_Reports").Cells("53", "C").Resize(3, 120).Copy 'D&A
Workbooks(ModelFileName).Worksheets(TabCopy).Cells("46", "AW").Resize(3, 120).PasteSpecial xlPasteValues
Application.CutCopyMode = False
Workbooks(NameFileOpen).Close
Application.DisplayAlerts = True
End Sub
- Batch Import Sub
although it only shows 7 callings, I have 27 calling in my file
Sub batch_import()
With Application
Call Import_NJ
Call Import_MD
Call Import_PA
Call Import_OKC
Call Import_CA
Call Import_HI
Call Import_IN
End With
Application.Calculation = xlCalculationAutomatic
ActiveWorkbook.Save
Application.DisplayAlerts = True
MsgBox _
("Batch loading Completed.")
End Sub
What I tried:
Turn off the automatic calculation in each Sub, as you see in the first example Macro. and also others application as many as I could.
I didn't shut down the screen updating since my manager wants to see it.
I activate the automatic calculating at the end of the Patch sub.
I guess the reseason slowing down the whole process is that I have more than 27 subs in the module.Also, there are a bunch of formulas filled in the worksheets.
Are there any ways to speed up the Macro regarding opening the file and running it? Let me know if I need to elaborate more on this question. Thanks for in advance and read through my question. : )
Update:08/15/2017
Because the Excel layout in the previous pic was in order, so Macro could search the last row and work perfectly. But in the real format, it's like this. Any thoughts I could alter it to the for loop?
1 Answer 1
The slowest part of your macro will be the opening of each Excel spreadsheet file. The copying part will run very quickly. Since the logic of copying from an external workbook to a sheet within the current workbook is consistent, only the locations and filenames are changing, it's easier to create a common method with parameters to normalize what's being done. That's what you'll see in the example below.
First, some general comments:
- You've got a single long line to declare all your variables. In VBA it's more common to declare each variable on a separate line. The reason is that it's easy to forget to declare a variable type when it's in a long line of
Dims
. The one you miss will default toVariant
, which, while not a bad thing, is not the desired declaration. - Always use
Option Explicit
to force VBA to check all variables for proper declaration. - Using the keyword
Let
is not required to set the value of a variable, however - There is a difference between a typed variable (Integer, Long, String, Double, etc) and an object variable (such as Range, Worksheet, Workbook, or a custom class). You only need to use the
Set
keyword in order to assign a "value" to an object. No keyword is necessary for a simple typed variable. - It may seem like more work to type in, but your code becomes cleaner and clearer if you specifically establish clearly named variables for the "source" and "destination". It's easier to keep track of where things are going. In fact, you should make it a habit to always define a variable for the
Workbook
and for theWorksheet
you're operating with.
For the main routine, you don't need to use a column to identify the row number. That can be determined programmatically at run time.
Option Explicit
Sub BatchImport()
Dim wb As Workbook
Dim ws As Worksheet
Dim lastRow As Long
Set wb = ThisWorkbook
Set ws = wb.Sheets("Control_Table")
With ws
lastRow = .Cells(.Cells.Rows.Count, 1).End(xlUp).Row
Dim i As Long
Dim path As String
Dim filename As String
Dim ext As String
Dim tabName As String
For i = 2 To lastRow
path = .Cells(i, 1)
filename = .Cells(i, 2)
ext = .Cells(i, 3)
tabName = .Cells(i, 10)
ImportStateData path, filename, ext, tabName
Next i
End With
wb.Save
MsgBox "Batch loading completed.", vbInformation + vbOKOnly, "Import Completed"
End Sub
Notice the parameters that are loaded from the columns on your Control_Table
and how they're passed to the working Sub
. The ImportStateData
sub accepts those parameters to change where the data comes from and goes to.
The biggest difference from your original code is how the copy-paste is accomplished. Because a copy and a paste is a notion from the manual actions a user would take to get the data into their destination, it's natural to want to use that function in your VBA. It's really not necessary, especially when you set up the ranges for the source and destination as shown here.
Private Sub ImportStateData(ByVal path As String, _
ByVal filename As String, _
ByVal ext As String, _
ByVal tabName As String)
Dim thisWB As Workbook
Dim destWS As Worksheet
Set thisWB = ThisWorkbook
Set destWS = thisWB.Sheets(tabName)
Dim stateWB As Workbook
Dim stateWS As Worksheet
Set stateWB = Workbooks.Open(path & "\" & filename & ext, _
UpdateLinks:=False, _
ReadOnly:=True)
Set stateWS = stateWB.Sheets("Total_Reports")
Dim stateRevenues As Range
Dim stateProdCosts As Range
Dim stateEmplMaint As Range
Dim stateDA As Range
Set stateRevenues = stateWS.Range("C9").Resize(5, 120)
Set stateProdCosts = stateWS.Range("C18").Resize(4, 120)
Set stateEmplMaint = stateWS.Range("C25").Resize(26, 120)
Set stateDA = stateWS.Range("C53").Resize(3, 120)
Dim destRevenues As Range
Dim destProdCosts As Range
Dim destEmplMaint As Range
Dim destDA As Range
Set destRevenues = destWS.Range("AW4").Resize(5, 120)
Set destProdCosts = destWS.Range("AW11").Resize(4, 120)
Set destEmplMaint = destWS.Range("AW17").Resize(26, 120)
Set destDA = destWS.Range("AW46").Resize(3, 120)
destRevenues.Value = stateRevenues.Value
destProdCosts.Value = stateProdCosts.Value
destEmplMaint.Value = stateEmplMaint.Value
destDA.Value = stateDA.Value
stateWB.Close
End Sub
So, for convenience here's the whole module in a single block:
Option Explicit
Sub BatchImport()
Dim wb As Workbook
Dim ws As Worksheet
Dim lastRow As Long
Set wb = ThisWorkbook
Set ws = wb.Sheets("Control_Table")
With ws
lastRow = .Cells(.Cells.Rows.Count, 1).End(xlUp).Row
Dim i As Long
Dim path As String
Dim filename As String
Dim ext As String
Dim tabName As String
For i = 2 To lastRow
path = .Cells(i, 1)
filename = .Cells(i, 2)
ext = .Cells(i, 3)
tabName = .Cells(i, 10)
ImportStateData path, filename, ext, tabName
Next i
End With
wb.Save
MsgBox "Batch loading completed.", vbInformation + vbOKOnly, "Import Completed"
End Sub
Private Sub ImportStateData(ByVal path As String, _
ByVal filename As String, _
ByVal ext As String, _
ByVal tabName As String)
Dim thisWB As Workbook
Dim destWS As Worksheet
Set thisWB = ThisWorkbook
Set destWS = thisWB.Sheets(tabName)
Dim stateWB As Workbook
Dim stateWS As Worksheet
Set stateWB = Workbooks.Open(path & "\" & filename & ext, _
UpdateLinks:=False, _
ReadOnly:=True)
Set stateWS = stateWB.Sheets("Total_Reports")
Dim stateRevenues As Range
Dim stateProdCosts As Range
Dim stateEmplMaint As Range
Dim stateDA As Range
Set stateRevenues = stateWS.Range("C9").Resize(5, 120)
Set stateProdCosts = stateWS.Range("C18").Resize(4, 120)
Set stateEmplMaint = stateWS.Range("C25").Resize(26, 120)
Set stateDA = stateWS.Range("C53").Resize(3, 120)
Dim destRevenues As Range
Dim destProdCosts As Range
Dim destEmplMaint As Range
Dim destDA As Range
Set destRevenues = destWS.Range("AW4").Resize(5, 120)
Set destProdCosts = destWS.Range("AW11").Resize(4, 120)
Set destEmplMaint = destWS.Range("AW17").Resize(26, 120)
Set destDA = destWS.Range("AW46").Resize(3, 120)
destRevenues.Value = stateRevenues.Value
destProdCosts.Value = stateProdCosts.Value
destEmplMaint.Value = stateEmplMaint.Value
destDA.Value = stateDA.Value
stateWB.Close
End Sub
-
\$\begingroup\$ Thanks for the cleaner format. It's much simpler. But what if after I change all of my code into this, which is processing faster once I execute the Macro, it is still running slow as opening up. My screen would freeze showing that "Opening: HsTbarI(100%). After 5 more mins, I would see Calculating: (4Processors(4)): x% trying to run as fast as it can on the bottom right of the status bar. \$\endgroup\$Oliver Wu– Oliver Wu2017年08月15日 14:30:09 +00:00Commented Aug 15, 2017 at 14:30
-
\$\begingroup\$ I didn't add the
Application.Calculation = xlCalculationManual
to my example macro, but you certainly can do that to help speed things up if your spreadsheets are doing that much heavy calculating on opening. But as I mentioned, you will always have the file opening time as the longest points in your program. \$\endgroup\$PeterT– PeterT2017年08月15日 14:37:09 +00:00Commented Aug 15, 2017 at 14:37 -
\$\begingroup\$ Gotcha! I do have lots of heavy calculations in the file. So I guess if I delete all of my giant Macro, it is still going to be slow as opening file since lots of heavy calculation? Unless I turn off the automatical calculation first? \$\endgroup\$Oliver Wu– Oliver Wu2017年08月15日 14:42:37 +00:00Commented Aug 15, 2017 at 14:42
-
\$\begingroup\$ Your giant macro is inefficient in terms of organization, which is why I illustrated how to collapse it to a single
Sub
with parameters. Performing the copy using the example method may give some slight speed up as well. But as I've said, your biggest time constraint is opening the files. Turn off the calculations and that's the best you can do. \$\endgroup\$PeterT– PeterT2017年08月15日 14:46:27 +00:00Commented Aug 15, 2017 at 14:46 -
\$\begingroup\$ I feel you. Instead of 27
sub
, I use only one sub with parameters to loop through the files. That would run faster and more efficient and organized! Much appreciated for the valuable lesson \$\endgroup\$Oliver Wu– Oliver Wu2017年08月15日 16:10:20 +00:00Commented Aug 15, 2017 at 16:10
Evaluate
the range you want to copy. this way you do not need to completely open the file (which is MUCH faster) ;) \$\endgroup\$sheet.evaluate()
which is able to return arrays (like a rangeA1:C7
) \$\endgroup\$