0

I have a code to load data from any number of workbooks we select and load in the current workbook. It works great in isolation (in a file where I dont perform any other tasks). However, when I used this code in a big file where I use(reference) the copied data in a number of array functions it takes over twenty minutes to load 1-2 files compared to seconds previously.

Is it possible its slow because of links to other tabs with functions? Am I missing something. Any help will be appreciated.

Application.ScreenUpdating = False
Application.DisplayAlerts = False
Application.AskToUpdateLinks = False
Application.Calculation = xlCalculationManual

Number = 0
IT = 0
Set thisWb = ActiveWorkbook
Set ws = thisWb.Sheets("CF")
thisWb.Sheets("CF").Select
ws.Range(ws.Cells(2, 1), ws.Cells(100000, 42)).ClearContents

Do
    files = Application.GetOpenFilename(filefilter:="Excel workbooks (*.csv*),*.csv*", Title:="Select files to import", MultiSelect:=True)
    If Not IsArray(files) Then Exit Sub 'Cancel must have been clicked
    If UBound(files) < 1 Then
        MsgBox "You have not selected any file. Please select files."
        End If
Loop Until UBound(files) > 0

 Number = UBound(files)
 N = Number + N

 For IT = 1 To UBound(files)
    Workbooks.Open files(IT)
    With ActiveWorkbook
        Application.CutCopyMode = False
        Set wk = ActiveWorkbook.ActiveSheet
        .ActiveSheet.Range("A2:AP10000").Copy
        'LastRow = wk.Cells(Rows.Count, "A").End(xlUp).Row
        thisWb.Activate
        ws.Select
        LastRow = ActiveSheet.Cells(Rows.Count, "A").End(xlUp).Row + 1
        Range("A" & LastRow).Select
        Set Rng = ws.Range("A" & LastRow)
        Rng.PasteSpecial xlPasteValues
        LastRow = ActiveSheet.Cells(Rows.Count, "A").End(xlUp).Row + 1
        Application.CutCopyMode = False
        .Close False
   End With
Next

Anything that can make this code run faster like load 3-4 small files in a minute will be perfect.

Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
Sid
  • 3
  • 3
  • You could start with avoiding to use `.Select` or `.Activate`, is not necessary. You can also do `someDestinationRange.value = someSourceRange.value`, if you want the values only, and don't care about formatting. – FAB Jun 24 '19 at 18:35
  • Thanks for the suggestions. Although, everytime I remove activate/select it starts giving object not found error so I couldnt do that. And I have heard of destination.value = source.value but how does it work if data is in different workbook as I tried equating the ranges of source and destination and it gave error and even that took time to reach there. – Sid Jun 24 '19 at 18:43
  • Just an example `thisWb.Sheets("CF").Select ws.Range(ws.Cells(2, 1), ws.Cells(100000, 42)).ClearContents` can be done as `ws.Range(ws.Cells(2, 1), ws.Cells(100000, 42)).ClearContents`. This is because you've already declared and set the worksheet variable, all you have to do is use it wisely. If you make sure you declare your variables appropriately, then you can do something like `FirstWS.Range("A1:A10").value = SecondWS.Range("C1:C10")`. One thing to keep in mind with this is that it works even if the ranges are not necessarily equal (which helps in certain situations). – FAB Jun 24 '19 at 19:18

1 Answers1

0

Here is an example of how to create variables and objects to keep track of which workbook and worksheet and data source you're using. Note also that I'm copying the data from a Range into a memory-based array for tons of speed.

Note also that it's highly recommended to always use Option Explicit.

Option Explicit

Sub test()
    Dim number As Long
    Dim it As Long
    number = 0
    it = 0

    Dim thisWB As Workbook
    Dim ws As Worksheet
    Set thisWB = ActiveWorkbook
    Set ws = thisWB.Sheets("CF")

    '--- clear the worksheet
    ws.Cells.Clear

    Dim files As Variant
    Do
        files = Application.GetOpenFilename(filefilter:="Excel workbooks (*.csv*),*.csv*", _
                                            Title:="Select files to import", _
                                            MultiSelect:=True)
        If Not IsArray(files) Then Exit Sub      'Cancel must have been clicked
        If UBound(files) < 1 Then
            MsgBox "You have not selected any file. Please select files."
        End If
    Loop Until UBound(files) > 0

    Dim n As Long
    number = UBound(files)

    Dim csvWB As Workbook
    Dim csvWS As Worksheet
    Dim csvData As Variant
    Dim dataRange As Range
    Dim lastRow As Long
    Dim rng As Range
    For it = 1 To UBound(files)
        Set csvWB = Workbooks.Open(files(it))
        With csvWB
            Set csvWS = csvWB.Sheets(1)
            csvData = csvWS.UsedRange                   'copy to memory-based array
            'Set csvData = csvWS.Range("A2:AP10000")    'copy to memory-based array
            Set dataRange = ws.Range("A1").Resize(UBound(csvData, 1), UBound(csvData, 2))
            dataRange.Value = csvData
            .Close False
        End With
    Next
End Sub
PeterT
  • 8,232
  • 1
  • 17
  • 38
  • I totally agree with using arrays when there is something to do with the data, but for copying only? I can't see how getting the data `source -> memory -> destination` is any better than `source -> destination`. Am I missing something? – FAB Jun 24 '19 at 19:10
  • For copying only (not transferring formats), source-->memory-->destination is MUCH faster. This is because the direct source-->destination action involves the worksheet `Range` objects and each individual cell that is copied invokes a recalculation of the entire worksheet. Yes, you can set calculations to `xlCalculationManual` but there are other events and object interactions that are triggered. Going the memory array route will always give you top speed in this regard. – PeterT Jun 24 '19 at 20:08
  • Thanks for the reply. The code seems more efficient although still takes a lot of time to copy when more than one file is selected. It could be an inherent issue with heavier function oriented tabs. But I am at loss to find why is it happening when I only copy data in manual mode. – Sid Jun 24 '19 at 20:29
  • Your code will spend the greatest amount of time in opening and closing the CSV files. That file operation in Excel causes lots of initialization of a new (empty) workbook and the importing of the CSV data into the worksheet, along with other things. The actual data copy will be done in a small fraction of the time. – PeterT Jun 24 '19 at 20:53
  • @PeterT thanks for that explanation... it makes sense i guess, I'm probably yet to copy large enough data to feel the difference. If the CSV is so large, would a `ADODB.Connection` be a better idea in this case? – FAB Jun 25 '19 at 08:49
  • I would think you will still be constrained by the file system because in all cases the file has to be opened somehow. Some connection types might skip some initialization, so you can try an `ADODB.Connection` (see [this answer](https://stackoverflow.com/a/1576687/4717755) for an interesting discussion). If the slowness of the CSV import or the ADODB connection becomes a factor, you can consider opening the file as a simple text file and parsing the lines in the code (which you're practically doing already). That may be the fastest option. – PeterT Jun 25 '19 at 12:56
  • Thanks everyone for the feedback! I definitely learned a number of new things about how vba code works. – Sid Jun 25 '19 at 17:37