Copy cells from multiple excel files and paste them into master file

Question

I got this VBA Code which is supposed to read out the cells from closed excel files (which are located in one folder) and copy the content into the master file. It seems to read out the files as supposed however pasting the copied contend seems not to work.

Any ideas?

Sub ReadAndMerceData()

Dim objFs As Object
Dim objFolder As Object
Dim file As Object

Set objFs = CreateObject("Scripting.FileSystemObject")
Set objFolder = objFs.GetFolder("C:\Users\XXX\Desktop\TEST")

Dim iStartRow As Integer
iStartRow = 0

For Each file In objFolder.Files

    Dim src As Workbook
    Set src = Workbooks.Open(file.Path)

    Dim iTotalRows As Integer
    iTotalRows = 50

    Dim iTotalCols As Integer
    iTotalCols = 17
    Dim iRows, iCols As Integer

    For iRows = 1 To iTotalRows
        For iCols = 1 To iTotalCols
            Cells(iRows + iStartRow, iCols) = src.Worksheets("Tabelle1").Cells(iRows, iCols)
        Next iCols
    Next iRows

    iStartRow = iRows + 1
    iRows = 0

    src.Close False
    Set src = Nothing
Next

End Sub

You should qualify the worksheet/workbook that `Cells(iRows + iStartRow, iCols)` are on/in. And you don't need to loop here either. — BigBen, Feb 11 '20 at 15:13
Note that if you declare `Dim iRows, iCols As Integer` only `iCols` is declared as `Integer` but `iRows As Variant`. In VBA you must specify a type for **every** variable otherwise it is `Variant` by default. Also these variables must be of type `Long` Excel has more rows than `Integer` can handle: `Dim iRows As Long, iCols As Long`. I recommend always to use `Long` instead of `Integer` as there is [no benefit in using `Integer` in VBA](https://stackoverflow.com/questions/26409117/why-use-integer-instead-of-long/26409520#26409520). — Pᴇʜ, Feb 11 '20 at 15:22
@BigBen the idea was to copy all excel files in a specific folder into the master excel. — Vincenzo, Feb 12 '20 at 07:58

Pᴇʜ · Accepted Answer · 2020-02-12T08:48:59.637

3

You don't need to copy over cell by cell. You can copy over the whole range at once, which is a lot faster.

Also make sure you specify the workbook and worksheet you want to copy into. Never use Range or Cells without specifing the worksheet (or Excel will guess and it might be wrong).

Option Explicit

Public Sub ReadAndMerceData()
    Dim objFs As Object        
    Set objFs = CreateObject("Scripting.FileSystemObject")

    Dim objFolder As Object
    Set objFolder = objFs.GetFolder("C:\Users\XXX\Desktop\TEST")

    Dim dest As Worksheet 'define your destination sheet!
    Set dest = ThisWorkbook.Worksheets("DestinationSheet")

    'make them variabes if they are dynamic otherwise use constants if hardcoded.
    Const TotalRows As Long = 50
    Const TotalCols As Long = 17 

    Dim iStartRow As Long

    Dim file As Object
    For Each file In objFolder.Files
        Dim src As Workbook
        Set src = Workbooks.Open(file.Path)

        'copy all cells at once
        dest.Cells(iStartRow + 1, 1).Resize(TotalRows, TotalCols).Value = src.Worksheets("Tabelle1").Cells(1, 1).Resize(TotalRows, TotalCols).Value

        iStartRow = iStartRow + TotalRows + 1

        src.Close SaveChanges:=False
    Next file
End Sub

Explanation

This dest.Cells(iStartRow + 1, 1) is the first cell we want to copy into so with .Resize(TotalRows, TotalCols) we expand that cell into a range and set its .Value equal to the source sheets range which starts in the first cell src.Worksheets("Tabelle1").Cells(1, 1) and has the same amount of rows and coluns .Resize(TotalRows, TotalCols).

Note that copying a full range is always faster than copying the same data cell by cell, because it is only 1 copy action that has to be performed.

edited Feb 12 '20 at 08:48

answered Feb 11 '20 at 15:32

Pᴇʜ

56,719
10
49
73

Hey @Pᴇʜ this is so awesome once again! You are great. Thank you so much. Specifically the explanation is very helpful. It makes sense that a whole range is faster to copy than many cells but I did not know that. Why is setting Set src = Nothing not necessary with your script? Is it set to nothing by SaveChanges:=False? – Vincenzo Feb 12 '20 at 08:22
1

@VincenzoKöstler I removed `Set src = Nothing` because I think it doesn't make any difference because you re-set it anyway in each iteration of the loop `Set src = Workbooks.Open(file.Path)` and it will automatically be destroyed at `End Sub`. • And I only changed it to `src.Close SaveChanges:=False` because of better human readabilty than just `src.Close False` where you have to know what `False` doing but if you read `SaveChanges:=False` it is very clear without thinking. Better readability = less errors (thats all, no magic here sorry ;) – Pᴇʜ Feb 12 '20 at 08:25
Hey @Pᴇʜ Oh I see, yeah that makes sense. I found the `source.Close False` somewhere online and actually did not understood what the False was doing. Now I do know :D Thanks! – Vincenzo Feb 12 '20 at 08:31
1

@VincenzoKöstler Actually that is exactly why I always like to write it down, because I'm lazy and I don't *want* to remember. If written down I can just read it like a book without having to think. Makes it easier to debug and understand the code later. – Pᴇʜ Feb 12 '20 at 08:37
Indeed very clever. Makes it easier if you come back later to reuse code as well I guess. Overall, you shortened the Code by **a lot** but it is still easy to read and understand I really like that. – Vincenzo Feb 12 '20 at 08:44
1

@VincenzoKöstler Actually I was not very consequent you can even remove `iStartRow = 0` because it is always `0` after `Dim iStartRow As Long`. I just fixed that. Now the code is even more clean. • Also declaring variables as close as possible to their first use (instead of all `Dim` statements at the top) can improve readability I think. Many people consider this to be a good coding style praxis. – Pᴇʜ Feb 12 '20 at 08:49
1

That is what I was told once but you took it further and put the `Dim` right above the `Set`. It looks really clean that way, I will do that in the future. Thank you very much again! – Vincenzo Feb 12 '20 at 10:07

score 2 · Answer 2 · answered Feb 11 '20 at 15:28

Foloowing @BigBen and also @Pᴇʜ suggestions, and also ordering your code a little to be more efficient, try the modified code below:

Option Explicit

Sub ReadAndMerceData()

' Objects and parameters declaration section
Dim objFs As Object
Dim objFolder As Object
Dim file As Object
Dim src As Workbook
Dim wb As Workbook
Dim ws As Worksheet
Dim iStartRow As Long, iTotalRows As Long, iTotalCols As Long, iRows As Long, iCols As Long

Set objFs = CreateObject("Scripting.FileSystemObject")
Set objFolder = objFs.GetFolder("C:\Users\XXX\Desktop\TEST")

' remove screen flickering (speed your code's run-time)
Application.ScreenUpdating = False

' set the result worknook and worksheet objects (modify to suit your needs)
Set wb = ThisWorkbook
Set ws = wb.Worksheets("sheet1") ' <-- modify "Sheet1" to your sheet's name

' set your parameters once, don't need to set them every time inside the loop
iStartRow = 0
iTotalRows = 50
iTotalCols = 17
For Each file In objFolder.Files
    Set src = Workbooks.Open(file.Path)

    For iRows = 1 To iTotalRows
        For iCols = 1 To iTotalCols
            ws.Cells(iRows + iStartRow, iCols) = src.Worksheets("Tabelle1").Cells(iRows, iCols)
        Next iCols
    Next iRows

    iStartRow = iRows + 1
    iRows = 0

    src.Close False
    Set src = Nothing
Next

Application.ScreenUpdating = True

End Sub

@Pᴇʜ agree (+1) .Still, I am trying to answer the original PO issue, it seems the past years it's a competition who write the best code (for free) while the people that raise the posts can barely use the MACRO recorder. — Shai Rado, Feb 11 '20 at 15:56
Good to hear :) I will delete all of my comments to tidy up here. — Pᴇʜ, Feb 11 '20 at 16:31
Hey guys, @ShaiRado sorry for not responding earlier. I really appreciate your help and all the support your are giving to people like me. It is never my intention to get anyone to code for me for free. I want to learn and to understand how the things are working and your answers help a lot doing that. Hopefully one day I can help out people the same way you are helping me out rn :) I am really thankful for the effort you put into the solution, trying to help me understand better what is going on. Thank you very much for that! — Vincenzo, Feb 12 '20 at 08:02

Copy cells from multiple excel files and paste them into master file

2 Answers2

Explanation