• Hi All

    Please note that at the Chandoo.org Forums there is Zero Tolerance to Spam

    Post Spam and you Will Be Deleted as a User

    Hui...

  • When starting a new post, to receive a quicker and more targeted answer, Please include a sample file in the initial post.

Question about Chandoo example macro: Consolidate data from different excel files (VBA)

P

PipBoy808

Member
Sub GetData()
Dim strWhereToCopy As String, strStartCellColName As String
Dim strListSheet As StringstrListSheet = “List”

On Error GoTo ErrH
Sheets(strListSheet).Select
Range(“B2″).Select
‘this is the main loop, we will open the files one by one and copy their data into the masterdata sheet
Set currentWB = ActiveWorkbook
Do While ActiveCell.Value <> “”
strFileName = ActiveCell.Offset(0, 1) & ActiveCell.Value
strCopyRange = ActiveCell.Offset(0, 2) & “:” & ActiveCell.Offset(0, 3)
strWhereToCopy = ActiveCell.Offset(0, 4).Value
strStartCellColName = Mid(ActiveCell.Offset(0, 5), 2, 1)
Application.Workbooks.Open strFileName, UpdateLinks:=False, ReadOnly:=True
Set dataWB = ActiveWorkbook
Range(strCopyRange).Select
Selection.Copy
currentWB.Activate
Sheets(strWhereToCopy).Select
lastRow = LastRowInOneColumn(strStartCellColName)
Cells(lastRow + 1, 1).Select
Selection.PasteSpecial xlPasteValues, xlPasteSpecialOperationNone
Application.CutCopyMode = False
dataWB.Close False
Sheets(strListSheet).Select
ActiveCell.Offset(1, 0).Select
Loop
Exit Sub
ErrH:
MsgBox “It seems some file was missing. The data copy operation is not complete.”
Exit Sub
End Sub

Hello. The above is an example from this link in which a bunch of data is consolidated within one sheet using VBA. My question concerns the line in red. What is it trying to accomplish? To me it looks like an offset(1,1) but that doesn't really make sense if you read the explanation in the link. Can anyone help?

Thanks!
 
The lastRow variable tells you where the last cell with data is in any column, as determined in the previous line. Note that we don't know what column it's from, just the row. The Cells object takes 2 arguments, row and column. So, the line in red says to go to 1 row after row with data, column 1 (aka, col A).

In other words, we want to make sure that we pick the first completely blank row.

PS. I believe author of the post wrote the code the way he did so it's easier to understand and follow-along. But, if you want to speed things up, remember to add the Application.ScreenUpdating commands, and getting rid of the excessive "Select"s would be good as well.
 
@PipBoy808
Hi!
Not just anymore a newbie at these forums, more than 70 posts, so why not use the proper window to post code as to keep it indented and readable? Penultimate icon of the toolbar. Thank you.
Regards!
 
Hi

I have a bit of time and I am going to take the Pepsi Challenge to speed up the code in the blog post.

PS. I believe author of the post wrote the code the way he did so it's easier to understand and follow-along. But, if you want to speed things up, remember to add the Application.ScreenUpdating commands, and getting rid of the excessive "Select"s would be good as well.
Click to expand...
I agree getting rid of the Section is going to make this puppy fly in comparison.

There are some issues in the file which need addressing. The user hard codes a cell reference the data copied from. This is represented by $A$2. Now VBA doesn't care about absolute or relative position of a cell. It sees $A$2, $A2,A$2 and A2 as the same thing cell A2 So there is no need to use absolute cell referencing when you are about to feed this into vb. This issue gets compounded when the code attempts to decipher which column to push the data into, this puppy;

strStartCellColName = Mid(ActiveCell.Offset(0, 5), 2, 1)

is in the coding in order to pull Col A out of the file. But if you just use A2 then you only need to call the first character in the string. Or Better still just use Column Start"A". Or just ask for Column Start NUMBER. Not confusing as everyone knows Col A is Col 1. The 2 in A2 isn't used so I would just bypass it and that gets rid of an additional 2 lines of coding (below) without losing any functionality. Additionally another 2 lines can be swiped with the Copy variable but I will leave it in as it makes the code easier to read. There are a number of other lines which can go too but for now they stay.

We could also look at the relatively long variable names. Variables are meant to shorten code so you can save time typing, they may well make code run faster. But this, strStartCellColName is a lot of text to include time and again.
The original code selects sheets and moves between both Parent and Child workbook. This is unnecessary and adds time to the efficient running of a procedure. This can be removed.

Finally the Consolidate Data button in the file, when pressed, rather than kicking off the procedure, it opens the Chandoo Blog post. This is an error and needs correcting. Here is my take on the code. I have left a lot of stuff in for readability but if it was left up to me much would go.

Code: 
Option Explicit
 
Sub ConsolidateDta()
Dim i As Integer
Dim fil As String
Dim Col As String
Dim cpy As String
Dim ws As Worksheet
Dim twb As Workbook
 
Set ws = Sheet1 ' List sheet
 
Application.DisplayAlerts = False
Set twb = ThisWorkbook
On Error GoTo Err 'This is just in case a muppet mistypes a path or file name.
 
    For i = 2 To ws.Range("B65536").End(xlUp).Row 'Sheet1 is MasterSheet
        fil = ws.Range("C" & i) & ws.Range("B" & i) 'File Location plus XL name
        cpy = ws.Range("D" & i) & ":" & ws.Range("E" & i) 'Copy Range
        Col = Left(ws.Range("B" & i), 1) 'Col to paste to
        Workbooks.Open fil, 0, 1 'Open Read Only
        Range(cpy).Copy
        twb.Sheets(ws.Range("F" & i).Value).Cells(Rows.Count, Col).End(xlUp)(2).PasteSpecial 12 'Vals only
        ActiveWorkbook.Close False 'Close no save
    Next i
Application.DisplayAlerts = False
Exit Sub
Err: 'Mup Mup
MsgBox "The file " & ws.Range("b" & i) & " is missing. Operation incomplete."
End Sub


I realise only one or two people will actually read every word above and take the time to test the workings. This post is for you J File attached to show workings. I will put a link in the blog post to this thread.

Take care

Smallman
 

Attachments

  • VBAmacroCpyDataFiles.xlsm
    26.2 KB · Views: 130
@SirJB7
Hi, myself!
So long...
You're still wondering who that post was addressed to, aren't you? So am I.
Regards!
 
Smallman said:
I have a bit of time and I am going to ...
Click to expand...
Hi, Smallman!
I have a bit of time too and I am going to write...
Hope you don't care, do you? If so, sorry, I apologize, but I'll do it again whenever I have a bit of time.
Regards!
PS: If you judge that as superfluous and curious, let me tell you that I love posting superfluously and curiously from time to time.
 
Thanks a lot Smallman!

For i = 2 To ws.Range("B65536").End(xlUp).Row

Is this line saying to look at rows 2:65536 up to the last row with data in it? If so, why is there a 'B' in front of the '65536'? Would that not constrict the code to column B only?
 
Hi PipBoy

You are most welcome.

Is this line saying to look at rows 2:65536 up to the last row with data in it? If so, why is there a 'B' in front of the '65536'? Would that not constrict the code to column B only?
Click to expand...


Yes it would restrict the code to column B only but as column B is the column where the XL files are listed, this is as good a place as any to start and end the file count. One of the Columns has to be responsible for this task. The line is the start of an iteration which will go from Row 2 to the last used Row in Col B. Now if there were 10 files listed in Col B then the procedure would start in Col B and iterate from row 2 to 11 then it would stop. Opening and each file, performing a transfer of information and closing each file one by one. Does that help explain what is happening?

Take care

Smallman
 
That's a great explanation. Just to clarify, if I put this at the end of a long column reference (A1:A9999):


End(xlUp).Row

that tells Excel to only go up to the last cell with a value in it?

What if, for arguments sake, there are gaps in your data? So, there are values in A1:A10, then a blank in A11, then values in A12:A18, then a gap in A19. Will the code look at A1:A10 and then stop?
 
Hi Pipboy

Your comments are not strictly correct.

if I put this at the end of a long column reference (A1:A9999):End(xlUp).Row
that tells Excel to only go up to the last cell with a value in it?
Click to expand...
There are a myriad of ways to reference ranges. The method used in my procedure above only traps one number, the last used Cell in Column B. If there are gaps in the data those gaps will be ignored.

A more accurate way to trap a 'long column reference' would be like the following which is my personal favourite but is limited to 65536 rows (to be exact). It is a throw back to XL 03 but for my purposes it works for everything as these days the data is not so big.

Code: 
Dim rng as range
set rng = Range(“a1”, range(“a65536”).end(xlup))
 
'Or for larger Datasets
Set rng = range(“a1:a” & cells(rows.count,”a”).end(xlup).row)
 
'Or Cells Method
set rng = Range(cells(1, 1), cells(rows.count, 1).end(xlup))

And there are many many more!!! It gets really interesting when you have a full range somewhere in the spreadsheet and you want to base an unfilled column on the length of that range. You typically do this to fill formula or trap a range for some creative activity.

What if, for arguments sake, there are gaps in your data?
Click to expand...

If there are gaps in the data the document will fail. This sort of file works because of two things;

1. A clearly defined process

2. The discipline to follow the above

The discipline in the case of the XL file would be always including a full file path, XL file name and Range to copy and leaving no gaps in the same way a tabular database is structured. When you have discipline you can produce consistency. That is the very essence of a robust reporting process. Hope these lessons help.

Take care


Smallman
 
 
Hi Dwee

You really should start your own thread with a link to this thread.

To be honest I don't really like the way this file has been set up. I was just copying a template from earlier in the thread and applying some logic. I would prefer a little more about your problem.

For instance, if you want to copy data from a selection of files with the Supplier column from all files being consolidated into a Master sheet then perhaps I would suggest putting all the files in a dedicated folder, then cycling through those files and finding the supplier file then transposing that data and closing to move onto the next workbook. It is cleaner that way.

Something like this

Code: 
Option Explicit

Sub GetData()
    Const sPath = "C:\Users\Marcus\Test\" 'Change to suit
    Dim sFil As String
    Dim ws As Worksheet
    Dim col As Integer
    Dim wb As Workbook
    Set ws = Sheet1 'The sheetcode name you want to consolidate to

    sFil = Dir(sPath & "*.xl*")
    Do While sFil <> ""
        Set wb = Workbooks.Open(sPath & sFil)
        col = [A1:Gz1].Find("Supplier").Column
        Range(Cells(1, col), Cells(500, col)).Copy ws.Range("Iv1").End(xlToLeft)(, 2)
        wb.Close 0
        sFil = Dir
    Loop
End Sub

Note: I did not make the ranges dynamic (500 rows) only and 250 cols max.

Take care

Smallman
 
Smallman said:
Hi Dwee

You really should start your own thread with a link to this thread.

To be honest I don't really like the way this file has been set up. I was just copying a template from earlier in the thread and applying some logic. I would prefer a little more about your problem.

For instance, if you want to copy data from a selection of files with the Supplier column from all files being consolidated into a Master sheet then perhaps I would suggest putting all the files in a dedicated folder, then cycling through those files and finding the supplier file then transposing that data and closing to move onto the next workbook. It is cleaner that way.

Something like this

Code: 
Option Explicit

Sub GetData()
    Const sPath = "C:\Users\Marcus\Test\" 'Change to suit
    Dim sFil As String
    Dim ws As Worksheet
    Dim col As Integer
    Dim wb As Workbook
    Set ws = Sheet1 'The sheetcode name you want to consolidate to

    sFil = Dir(sPath & "*.xl*")
    Do While sFil <> ""
        Set wb = Workbooks.Open(sPath & sFil)
        col = [A1:Gz1].Find("Supplier").Column
        Range(Cells(1, col), Cells(500, col)).Copy ws.Range("Iv1").End(xlToLeft)(, 2)
        wb.Close 0
        sFil = Dir
    Loop
End Sub

Note: I did not make the ranges dynamic (500 rows) only and 250 cols max.

Take care

Smallman
Click to expand...

Hey i am quite to VBA. i tried using your code but it didnt work.I opened a blank file and save your code as the blank file's macro and saved it as an xlsm file and tried running but i an always stuck at the first file of my folder and then an error screen saying Object variable or With block variable not set. Help please.

Scottie
 
Smallman said:
Hi Dwee

You really should start your own thread with a link to this thread.

To be honest I don't really like the way this file has been set up. I was just copying a template from earlier in the thread and applying some logic. I would prefer a little more about your problem.

For instance, if you want to copy data from a selection of files with the Supplier column from all files being consolidated into a Master sheet then perhaps I would suggest putting all the files in a dedicated folder, then cycling through those files and finding the supplier file then transposing that data and closing to move onto the next workbook. It is cleaner that way.

Something like this

Code: 
Option Explicit

Sub GetData()
    Const sPath = "C:\Users\Marcus\Test\" 'Change to suit
    Dim sFil As String
    Dim ws As Worksheet
    Dim col As Integer
    Dim wb As Workbook
    Set ws = Sheet1 'The sheetcode name you want to consolidate to

    sFil = Dir(sPath & "*.xl*")
    Do While sFil <> ""
        Set wb = Workbooks.Open(sPath & sFil)
        col = [A1:Gz1].Find("Supplier").Column
        Range(Cells(1, col), Cells(500, col)).Copy ws.Range("Iv1").End(xlToLeft)(, 2)
        wb.Close 0
        sFil = Dir
    Loop
End Sub

Note: I did not make the ranges dynamic (500 rows) only and 250 cols max.

Take care

Smallman
Click to expand...

In fact, I have tried changing the value of col to 1 straightaway and my files have three rows of value for the column of "Supplier" which is also at column A therefore corresponding to the value of col which is 1. But there is no value being copied at all, the files only open and close with nothing done.
 
NARAYANK991 said:
Hi ,

Can you post the line of code which generated an error in your previous post ?

Narayan
Click to expand...
Hello! Thank you so much for replying.. i changed a line of the code from col = [A1:Gz1].Find("Supplier").Column to col = 1 and the error is gone.
However, the program did not copy any of the values under the column" Supplier" from the xlsx files in the folder.
 
Hi ,

Which column has the header Supplier ?

You can assign that column number to the variable col. For example , if column AF has the header label Supplier , you would assign the value 32 to col.

Narayan
 
NARAYANK991 said:
Hi ,

Which column has the header Supplier ?

You can assign that column number to the variable col. For example , if column AF has the header label Supplier , you would assign the value 32 to col.

Narayan
Click to expand...
Yes I did exactly that to test out that program. I have column A that has the header label Supplier therefore I assigned the value 1 to col. However, no data is being pasted.
 
Hi ,

Please upload your workbook with the data and the code in it.

And upload at least a few other files from which the consolidation is to be done.

Narayan
 
NARAYANK991 said:
Hi ,

Please upload your workbook with the data and the code in it.

And upload at least a few other files from which the consolidation is to be done.

Narayan
Click to expand...

Hello,
I have uploaded the files.
Scottie
 

Attachments

  • ExcelVBA.xlsm
    14.3 KB · Views: 22
  • Test1.xlsx
    8.1 KB · Views: 19
  • Test2.xlsx
    8.1 KB · Views: 14
  • Test3.xlsx
    8.1 KB · Views: 13
NARAYANK991 said:
Hi ,

See if this works.

Narayan
Click to expand...

hello Narayan, thanks so much. I am so sorry to disturb you but I wanna make this program even better by making it able to search for the column with "Supplier" as the column header and consolidate the data. It can't be that everytime the "Supplier"column is the 1st column, haha.
 
You must log in or register to reply here.
Back
Top