I have set up a folder and workbooks that match your description. The revised code below works for me. I hope the original code was all yours. I can easily forgive a newbie for some of the mistakes and bad practices but I would be appalled if you got this from a tutorial as you suggest.
I have included comments to explain all my changes. Come back for more explanations if necessary but the more you decipher my code for yourself, the faster you will develop your skills. I include comments to say what the code is doing but few comments about the statements themselves. For example, Option Explicit is easy to look up once you know it exists so its purpose is not explained.
Option Explicit ' Always have this statement at the top of every module
' Constants are fixed while a macro is running but can be changed
' if the data is redesigned. This defines the first data row of every
' worksheet is 2. That is, it allows for one header row. I could have used
' 2 within the code below. If you ever have to update code because the
' number of header rows has changed or a new column has been inserted in
' the middle of existing columns, you will understand why I use constants.
Const RowDataFirst As Long = 2 ' Set to 1 if no header rows
' You assume that when you open a workbook, the active worksheet is the one
' required. This is only reliable if the workbooks only have one worksheet.
' I have defined one constant for the name of the worksheet within the
' destination workbook and one for name of the worksheet within every
' source workbook. I assume this is adequate. I will have alternative
' suggestions if it is not adequate.
Const WshtDestName As String = "Data"
Const WshtSrcName As String = "Data"
Sub copyDataFromMultipleWorkbooksIntoMaster()
Dim FolderPath As String
Dim FilePath As String
Dim FileName As String
Dim HeaderCopied As Boolean
Dim RowDestNext As Long
Dim RngDest As Range
Dim RngSrc As Range
Dim WbkDest As Workbook
Dim WbkSrc As Workbook
Dim WshtDest As Worksheet
Dim WshtSrc As Worksheet
Application.ScreenUpdating = False
' You need row numbers in both the source and the destination worksheets.
' Use names for variables that tell you exactly what the variable is for.
' While you are writing a macro, it is easy to remember odd names but if
' you return to the macro in six or twelve months will you still remember?
' I have a naming system which I always use. I can look at macros I wrote
' ten years ago and know what all the variable are which is an enormous help
' when updating old code. If you do not like my system then develop your own.
' My names consist of a series of keywords with the most global first.
' "Row" says the variable is a row number. "Wbk" says the variable is a
' workbook. "RowXxx" says the variable is a row number within worksheet or
' array Xxxx. "RowSrcXxx" says the variable is a row number for worksheet
' "Source". "Xxx" can be "First", "Crnt", "Next", Prev", "Last" or whatever
' I need for the current macro
Dim RowSrcLast As Long
' My comment suggested you be consistent in your use of column numbers but
' comments do not allow enough room to explain. With some statements, having
' a different number of rows or columns in the source and destination can
' give funny results with truncation or duplication. If you know you only
' want 9 columns then use 9 in both source and destination ranges. If the
' number of columns might change then determine the number at runtime.
Dim ColSrcLast As Long
' If you are handling multiple workbooks be explicit which workbook
' you are addressing. This assumes the workbook into which the worksheets
' are collected is the workbook containing the macro.
Set WbkDest = ThisWorkbook
Set WshtDest = WbkDest.Worksheets(WshtDestName)
' Note a worksheet variable references the worksheet within its workbook.
' I do not need to write WbkDest.WshtDest.
' FolderPath = "C:\Users\PC-1\Desktop\Merge\"
' You can hard code the name of the folder into a macro but it is
' a bother when you move your workbooks. When all your workbooks
' are in the same folder, the following is more convenient
FolderPath = WbkDest.Path & "\"
FilePath = FolderPath & "*.xls*"
' Note Dir searches down the folder index for files that match the template.
' The sequence in which they are found depends on the sequence in which the
' files were added to the folder. There are other techniques if sequence is
' important.
FileName = Dir$(FilePath) ' Dir$ is marginally faster than Dir
' Your existing code adds new data to the end of the existing worksheet in
' Master.xlsm. This may be correct but it is more usual to clear the
' destination at the start of each run. Comment out the first block and uncomment
' the second block if you want to add to existing data.
With WshtDest
.Cells.EntireRow.Delete ' Delete every row in worksheet
HeaderCopied = False ' There is no header within the destination worksheet
RowDestNext = 1 ' First (only) header row will be copied from first
' source worksheet to this row
End With
' If you know that column A of the used rows of the active sheet contains no
' blank cells, the following is the easiest way of finding that last used row:
' RowDestLast = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row
' But this technique is unreliable if there might be blank cells. No technique
' is 100% reliable but you would have very strange data if the technique I have
' used is not reliable for you.
'With WshtDest
' ' Find last row with a value
' Set RngDest = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByRows, xlPrevious)
' If RngDest Is Nothing Then
' ' No data has been found so the worksheet is empty
' HeaderCopied = False ' There is no header within the destination worksheet
' RowDestNext = 1 ' First (only) header row will be copied from first
' ' source worksheet to this row
' Else
' ' There is data within the worksheet. Assume the header row(s) are present.
' HeaderCopied = True
' RowDestNext = RngDest.Row + 1
' End If
'End With
' Please indent your code within Do-Loops, If, etc. It makes your code
' much easier to read.
' All your workbooks are within the same folder. Master.xlsm will be one
' of those found but you do not want to use it as a source workbook.
Do While FileName <> ""
If FileName <> WbkDest.Name Then
Set WbkSrc = Workbooks.Open(FolderPath & FileName)
' WbkSrc will be the active workbook but better to reference it explicitly
With WbkSrc
Set WshtSrc = .Worksheets(WshtSrcName)
End With
With WshtSrc
' Find last row with data if any
Set RngSrc = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByRows, xlPrevious)
If RngSrc Is Nothing Then
' No data has been found so the worksheet is empty
Else
RowSrcLast = RngSrc.Row
' Find last column with data. Already know there is data
RngSrc = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByColumns, xlPrevious)
ColSrcLast = RngSrc.Column
If HeaderCopied Then
' Already have header row(s) in destination worksheet
Set RngSrc = .Range(.Cells(RowDataFirst, 1), .Cells(RowSrcLast, ColSrcLast))
Else
' Do not have header row(s) in destination worksheet. Include them in copy.
Set RngSrc = .Range(.Cells(1, 1), .Cells(RowSrcLast, ColSrcLast))
HeaderCopied = True
End If
RngSrc.Copy Destination:=WshtDest.Cells(RowDestNext, 1) ' Copy data and formats
RowDestNext = RowDestNext + RngSrc.Rows.Count ' Step ready for next copy
End If
End With ' WshtSrc
WbkSrc.Close SaveChanges:=False
Set WshtSrc = Nothing
Set WbkSrc = Nothing
End If ' FileName <> WbkDest.Name
FileName = Dir$
Loop ' While FileName <> "" And FileName <> WbkDest.Name
With WshtDest
.Cells.EntireColumn.AutoFit
End With
Application.ScreenUpdating = True
End Sub
New section in response to OP's comments on original answer
There are some things I really should have included in my original answer but you have also strayed into areas I could not have expected. You have also found a bug which my own testing missed.
“Was I supposed to substitute anything like my workbook(s) name …”
I should have made this clear. In Const WshtDestName As String = "Data", “Data” is my name for the worksheet in which I accumulate the data. I should have told you to replace “Data” with your name for the worksheet.
Your comments suggest you have replaced:
Set WshtDest = WbkDest.Worksheets(WshtDestName)
with
Set WshtDest = WbkDest.Worksheets("Sheet1")
If so, please update the Const statement instead. The objective of using Const statements is to isolate things that might change from the main body of the code. This makes maintenance easier.
Avoid using the default names “Sheet1”, “Sheet2” and so on. As your data and your macros get more complicated, it makes life much easier if worksheet names reflect the worksheet contents.
[Note from OP: I renamed my master wksht to 'Combined' and my source's wkshts to 'Node Export By Hub', and replaced the 'Sheet1' name in the Constants with those names.]
I use WbkDest.Name as the name of the master workbook. You do not need to change this to your actual workbook name. Using properties like this makes your code much easier to maintain because the property value will change if you rename the workbook.
I received a Run Time Error 9 Subscript out of Range error and when I debugged it, Set WshtDest = WbkDest.Worksheets(WshtDestName) was highlighted.
This paragraph may be beyond your current knowledge of VBA. Read it and extract what understanding you can. It will become clearer as you advance to arrays and collections. Worksheets is a Collection or what most programming languages call a list. A collection is like an array except you can add new values in the middle and delete existing values. Within arrays, entries are only accessible via an index number, for example: MyArray(5). Entries within a collection are accessible via an index but collection entries can also have a key. If I had written Worksheets(5) this would have given error 9 because there is no worksheet 5. When you ran the macro, WshtDestName had a value of “Data”. There was no Worksheets("Data") so you got error 9. If you update the Const statement, this error will go because Worksheets("Sheet1") exists.
I didn't know if the constants at the beginning were supposed to be after the Sub so I moved them to be after the Dim's under Sub and nothing happened then.
Here you have strayed into the topic of "Scope". If I declare a constant or a variable within a subroutine or a function, it is only visible within that subroutine or function. The “scope” of the constant or variable is the function. If I declare a constant or a variable at the top of a module, it is visible to every subroutine or function within that module but not to those in other modules. The “scope” of the constant or variable is the module. If I add “Public” to the declaration, the constant or variable is visible to every subroutine or function in every module or user form within the workbook. The “scope” of the constant or variable is the workbook.
[Note from OP: This is interesting to know because I couldn't find information about constants before the Sub via Google. Thank you.]
This workbook has only one subroutine and only one module and no user forms so it does not really matter where you place the constant declarations. The most appropriate scope for a constant or variable is a complicated issue and I am not going to attempt an introduction. I will just say that these constants are recording my assumptions about the workbook. If there had been multiple modules I would have defined them as Public.
You need to review all my assumptions. I do not have your data. I have made up data that I believe matches your data. But if any of my assumptions about your data are wrong, the macro will not work.
When I hit F8 to look at each line of code, here is what the screen tip says: For Application.ScreenUpdating = False it says Application.ScreenUpdating = True
When a statement is yellow, it has not yet been executed. With most statements, you can actually amend it before pressing F8 again to execute it. So if you have a yellow A = A + 2 and you think: “I meant A = A + 3”, you can correct the statement before execution.
True is the default value for Application.ScreenUpdating so that is the value you see before Application.ScreenUpdating = False is executed.
For Set WbkDest = ThisWorkbook the screen tip says for WbkSet = Nothing and for ThisWorkbook = , but only says these things when yellow highlighted. When hit F8 and move off that line, it doesn't say anything when I put the cursor over it.
If you hover over an ordinary variable (data type = Long, String, Boolean, etc.), the interpreter will display its value. An object (such as WbkDest) has lots of properties; which should the interpreter display? Some objects, such as Range, have a default property. For Range this is Value so if you hover over a range, you see the value. A workbook has no default property so the interpreter doesn’t display anything.
Go down to the Immediate Window and type ? WbkDest.Name and click Enter. The interpreter will display the workbook’s name. You can get the value of any of the workbook’s properties displayed. You can also display sub-properties, for example: WbkDest.Worksheets.Count or WbkDest.Worksheets("Sheet1").Range("A1").Value.
[Note from OP: 1st error - I received a Run-time '424': Object required error when I typed '? WbkDest.Name' and hit Enter in the Immediate Window. Is it not recognizing WbkDest because it is declared in the beginning Dim and later Set = This Workbook. I changed the name to my MASTER.xlsm to MASTER_DESKTOP TEST.xlsm but that shouldn't matter because we never explicitly mention it in this code, correct?]
Response from TD to above note Dim X As Type reserves some space for X and sets its value to the default for the type. For type Long, the value will be zero. For type Object (Workbook is a sub-type of Object), the default value is Nothing. Nothing does not have any properties so at this stage ? WbkDest.Name will give an error. When statement Set WbkDest = ThisWorkbook is executed, WbkDest now gives access to ThisWorkbook. ThisWorkbook has a Name so ? WbkDest.Name will have a value. You are correct; you can rename the workbook without changing the code.
Set RngDest = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByRows, xlPrevious) where RngDest = Nothing in tip and nothing pops up in screen tip except -4123 for xlFormulas, and 1 for xlByRows, and 2 for xlPrevious.
I deduce from this you want to add new data to the bottom of data from previous runs of the macro. In my experience this is unusual but I included the code for this option just in case.
[Note from OP: FYI, yes, the Master has headers and the data will be added below data that has been previously copied over via the macros.]
Response from TD to above note My code is more complicated than you need but includes the functionality you want. If the master worksheet is empty, the header row(s) and the data rows will be copied from the first source worksheet. Only the data rows will be copied from any other workbooks. If the master worksheet is NOT empty, only data rows are copied from the source worksheets.
xlFormulas, xlByRows and xlPrevious are Excel defined constants so the parameters for Find are meaningful names rather than strange numbers.
From the other statements listed, I deduce that the destination workbook is currently empty.
[Note from OP: FYI, yes, the Master/destination wkbk has a header row in the 1st row, but is otherwise empty to begin with.]
Response from TD to above note See my last response.
Do While FileName <> "" And FileName <> WbkDest.Name where both FileName and WbkDest.Name = "MASTER.xlsm" in screen tip.
F8 then jumps through the rest of the code to the end where
With WshtDest .Cells.EntireColumn.AutoFit End With, etc.
At this point you have hit a bug in my code. I do not understand why my testing did not encounter this bug.
You need to ask: why has the loop exited? Why hasn’t it repeated for the next file? If I leave out most of the code, you get:
Do While FileName <> "" And FileName <> WbkDest.Name
‘ Code to process interesting file
FileName = Dir$
Loop ' While FileName <> "" And FileName <> WbkDest.Name
FileName <> "" is True since FileName = “MASTER.xlsm” but FileName <> WbkDest.Name is False since “MASTER.xlsm” = WbkDest.Name. The end condition has been reached and the loop ends without checking any other files.
I should have written:
Do While FileName <> ""
If FileName <> WbkDest.Name
‘ Code to process interesting file
End If
FileName = Dir$
Loop ' While FileName <> ""
With this code, workbook “MASTER.xlsm” is ignored as required but the loop continues looking for further workbooks.
Amend the macro to match the revised structure and try again.
[Note from OP: 2nd Error I received a Compile Error - Expected: Then or Go To, so I just added Then after If FileName <> WbkDest.Name, so it reads
If FileName <> WbkDest.Name Then
Set WbkSrc=Workbooks.Open (FolderPath & FileName)
'Rest of Code
Is this correct?]
Response from TD to above note Yes you are correct. I should have included the tested code rather than trying to create a summary.
[Note from OP: 3rd error - After I added all of the edits, I ran the Compile VBA Project under Run and it said, Compile Error: Loop without Do', which I don't understand because the Loop I'm sure it is referring to is at the bottom and never had a 'Do' next to it. In other words, I'm not sure why it is raising an error now when it never had a 'Do'.]
Response from TD to above note Compile errors "Loop without Do", "Do without Loop", "If within End If", "End If without If", etc. can be confusing. Do loops, for loops and Ifs must be properly nested. If you fail to complete one structure completely, the compiler complains about the outer structure which is probably perfect. My guess, is you have not included the End If for the new If. When the compiler hits the Loop it is looking for the End If or the start of a nested structure. I have replaced my original code with my revised code which I have just tested again. You can copy the new code and update for your names. However, it may be better to work down your loop and match it against mine. My guess is you will find an End If in my code that is missing from yours.
SrcRange.Copy Destination:=DestTopLeftCellis the VBA equivalent. – Tony Dallimore