3
\$\begingroup\$

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
  1. 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:

  1. 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.

  2. I didn't shut down the screen updating since my manager wants to see it.

  3. 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?

enter image description here

asked Aug 14, 2017 at 16:59
\$\endgroup\$
6
  • \$\begingroup\$ So you're saying each of the 27 subs is identical except for the row number? \$\endgroup\$ Commented Aug 14, 2017 at 17:26
  • \$\begingroup\$ That's correct. I would manually change "2","D" to "3","D" etc. So the code can open an external file based on the reference. For example, D2 is NJ file. D3 is MD file. Let me know if that is understandable. \$\endgroup\$ Commented Aug 14, 2017 at 17:39
  • \$\begingroup\$ looking for speed, it may be much better to Evaluate the range you want to copy. this way you do not need to completely open the file (which is MUCH faster) ;) \$\endgroup\$ Commented Aug 15, 2017 at 12:18
  • \$\begingroup\$ @DirkReichel It sounds like there are ways of copying and pasting without opening files one by one? \$\endgroup\$ Commented Aug 15, 2017 at 14:21
  • \$\begingroup\$ like you would ref to a different workbook inside the sheet, you can also using sheet.evaluate() which is able to return arrays (like a range A1:C7) \$\endgroup\$ Commented Aug 15, 2017 at 14:24

1 Answer 1

2
\$\begingroup\$

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:

  1. 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 to Variant, which, while not a bad thing, is not the desired declaration.
  2. Always use Option Explicit to force VBA to check all variables for proper declaration.
  3. Using the keyword Let is not required to set the value of a variable, however
  4. 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.
  5. 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 the Worksheet 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
answered Aug 15, 2017 at 3:00
\$\endgroup\$
6
  • \$\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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented Aug 15, 2017 at 16:10

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.