PDA

View Full Version : Merging Data From Workbooks macro



phesago
05-08-2014, 10:35 AM
Hello All!


Having some issues with my macro. Wondering if any of you fine people would take a moment to see whats wrong with my code? Getting tons of awesome errors, and seeing in how this is my first macro, your expertise would be greatly appreciated! Thanks in advance, I cant wait to learn more! :)



Sub FAOName()
Const ColResultFaoName As Long = 1
Const FAOName As String = "FAO_Name"
Const WBkMasterName As String = "mater.data.xls"
Const WBkResultName As String = "results.data.xls"
Const WShtMasterName As String = "Combined"
Const WShtResultName As String = "FAO"
Dim ColMasterFaoName As Long
Dim ColMasterCrnt As Long
Dim CountMasterColFoundCrnt As Long
Dim CountMasterColFoundTotal As Long
Dim InxWbkCrnt As Long
Dim PathCrnt As String
Dim RngResult As Range
Dim RowMasterNext As Long
Dim RowResultLast As Long
Dim TempStg As String
Dim WBkMaster As Workbook
Dim WBkResult As Workbook
Dim WShtMaster As Worksheet
Dim WShtResult As Worksheet
PathCrnt = ThisWorkbook.Path
For InxWbkCrnt = 1 To Workbooks.Count
If Workbooks(InxWbkCrnt).Name = WBkMasterName Then
Call MsgBox("Please close workbook '" & WBkMasterName & _ "' before running this macro.", vbOKOnly)
Exit Sub
End If
Next
On Error Resume Next
Workbooks.Open PathCrnt & "\" & WBkMasterName
On Error GoTo 0
If ActiveWorkbook.Name = ThisWorkbook.Name Then
Call MsgBox('I was unable to open workbook " &_ WBkMasterName & "'.", vbOKOnly)
Exit Sub
End If
Set WBkMaster = ActiveWorkbook
On Error Resume Next
Workbooks.Open PathCrnt & "\" & WBkResultName
On Error GoTo 0
If ActiveWorkbook.Name = WBkMaster.Name Then
Call MsgBox("I was unable to open workbook '" &_ WBkResultName & "'.", vbOKOnly)
WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
Set WBkResult = ActiveWorkbook
With WBkMaster
On Error Resume Next
Set WShtMaster = .Worksheets(WShtMasterName)
On Error GoTo 0
If WShtMaster Is Nothing Then
Call MsgBox("Workbook '" & WBkMasterName & "' does not contain " &_ "worksheet '" & WShtMasterName & "'.", vbOKOnly)
WBkMaster.Close SaveChanges:=False
WBkResult.Close SaveChanges:=False
Set WBkMaster = Nothing
Set WBkResult = Nothing
Exit Sub
End If
End With
With WBkResult
On Error Resume Next
Set WShtResult = .Worksheets(WShtResultName)
On Error GoTo 0
If WShtResult Is Nothing Then
Call MsgBox("Workbook '" & WBkResultName & "' does not contain " & _ "worksheet '" & WShtResultName & "'.", vbOKOnly)
WBkMaster.Close SaveChanges:=False
WBkResult.Close SaveChanges:=False
Set WBkMaster = Nothing
Set WBkResult = Nothing
Exit Sub
End If
End With
With WShtResult
Debug.Assert .Cells(1, ColResultFaoName).Value = ColFaoName
ColResultFaoName = "A"
If .Cells(1, ColResultFaoName).Value <> ColFaoName Then
Call MsgBox("Cell " & Replace(.Cells(1, ColResultFaoName).Address, "$", "") & " of worksheet'" & WShtResultName & "' of workbook '" & _ WBkResultName & "' is not " & ColFaoName & ".", vbOKOnly)
WBkMaster.Close SaveChanges:=False
WBkResult.Close SaveChanges:=False
Set WBkMaster = Nothing
Set WBkResult = Nothing
Exit Sub
End If
End With
With WShtResult
RowResultLast = .UsedRange.Row + .UsedRange.Rows.Count - 1
Set RngResult = .Range(.Cells(2, ColResultFaoName), .Cells(RowResultLast, ColResultFaoName))
End With
With WShtMaster
RowMasterNext = .UsedRange.Row + .UsedRange.Rows.Count
RngResult.Copy Destination:=.Cells(RowMasterNext, ColMasterFaoName)
End With
WBkMaster.Close SaveChanges:=True
WBkResult.Close SaveChanges:=False
End Sub

ranman256
05-09-2014, 12:29 PM
"On Error Resume Next" should appear 1st and never again

No need to CALL a msgbox...just loose the parenthesis , (and the CALL)
MsgBox "Please close workbook '" & WBkMasterName & "' before running this macro.", vbOKOnly
...this will show you the error in syntax '& _' , you only need the & .

p45cal
05-09-2014, 02:56 PM
"On Error Resume Next" should appear 1st and never againWell, perhaps…
The way phesago has those statements is around lines which are likely to throw an error, and while developing the code I wouldn't want to hide when errors occur anywhere else in the code. So I would advise phesago to keep his On Error statements as they are for the time being.

phesago, you have declared a variable ColMasterFaoName as Long but never assign anything to it (it holds 0). You use another undeclared variable called ColFaoName. Are these two meant to be one and the same?

I also note that if WkbookMaster is already open you ask the user to close it, then when the code runs again the code opens it again!
For the start of your macro you could avoid that by using it if it is open, flagging it as already open so that the code doesn't close it if it was already open if an error elsewhere is encountered:
Sub FAOName2()
Const ColResultFaoName As Long = 1
Const FAOName As String = "FAO_Name"
Const WBkMasterName As String = "mater.data.xls"
Const WBkResultName As String = "results.data.xls"
Const WShtMasterName As String = "Combined"
Const WShtResultName As String = "FAO"
Dim ColMasterFaoName As Long
Dim ColMasterCrnt As Long
Dim CountMasterColFoundCrnt As Long
Dim CountMasterColFoundTotal As Long
Dim InxWbkCrnt As Long
Dim PathCrnt As String
Dim RngResult As Range
Dim RowMasterNext As Long
Dim RowResultLast As Long
Dim TempStg As String
Dim WBkMaster As Workbook
Dim WBkResult As Workbook
Dim WShtMaster As Worksheet
Dim WShtResult As Worksheet
Dim WBkMasterWasOpen As Boolean, WBkResultWasOpen As Boolean
PathCrnt = ThisWorkbook.Path

On Error Resume Next
Set WBkMaster = Workbooks(WBkMasterName)
If WBkMaster Is Nothing Then
Set WBkMaster = Workbooks.Open(PathCrnt & "\" & WBkMasterName)
Else
'WBkMaster was open so flag not to close it later:
WBkMasterWasOpen = True
End If
On Error GoTo 0
If WBkMaster Is Nothing Then
MsgBox "I was unable to open workbook '" & WBkMasterName & "'.", vbOKOnly
Exit Sub
Else
With WBkMaster
On Error Resume Next
Set WShtMaster = .Worksheets(WShtMasterName)
On Error GoTo 0
If WShtMaster Is Nothing Then
MsgBox "Workbook '" & WBkMasterName & "' does not contain worksheet '" & WShtMasterName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
End With
End If

On Error Resume Next
Set WBkResult = Workbooks(WBkResultName)
If WBkResult Is Nothing Then
Set WBkResult = Workbooks.Open(PathCrnt & "\" & WBkResultName)
Else
'WBkResult was open so flag not to close it later:
WBkResultWasOpen = True
End If
On Error GoTo 0
If WBkResult Is Nothing Then
MsgBox "I was unable to open workbook '" & WBkResultName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
If Not WBkResultWasOpen Then WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
Exit Sub
Else
With WBkResult
On Error Resume Next
Set WShtResult = .Worksheets(WShtResultName)
On Error GoTo 0
If WShtResult Is Nothing Then
MsgBox "Workbook '" & WBkResultName & "' does not contain worksheet '" & WShtResultName & "'.", vbOKOnly
WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
End With
End If

'your final col/fao/name check
'your copying
'your saving

End Sub

phesago
05-10-2014, 05:02 PM
Well, perhaps…
The way phesago has those statements is around lines which are likely to throw an error, and while developing the code I wouldn't want to hide when errors occur anywhere else in the code. So I would advise phesago to keep his On Error statements as they are for the time being.

phesago, you have declared a variable ColMasterFaoName as Long but never assign anything to it (it holds 0). You use another undeclared variable called ColFaoName. Are these two meant to be one and the same?

I also note that if WkbookMaster is already open you ask the user to close it, then when the code runs again the code opens it again!
For the start of your macro you could avoid that by using it if it is open, flagging it as already open so that the code doesn't close it if it was already open if an error elsewhere is encountered:
Sub FAOName2()
Const ColResultFaoName As Long = 1
Const FAOName As String = "FAO_Name"
Const WBkMasterName As String = "mater.data.xls"
Const WBkResultName As String = "results.data.xls"
Const WShtMasterName As String = "Combined"
Const WShtResultName As String = "FAO"
Dim ColMasterFaoName As Long
Dim ColMasterCrnt As Long
Dim CountMasterColFoundCrnt As Long
Dim CountMasterColFoundTotal As Long
Dim InxWbkCrnt As Long
Dim PathCrnt As String
Dim RngResult As Range
Dim RowMasterNext As Long
Dim RowResultLast As Long
Dim TempStg As String
Dim WBkMaster As Workbook
Dim WBkResult As Workbook
Dim WShtMaster As Worksheet
Dim WShtResult As Worksheet
Dim WBkMasterWasOpen As Boolean, WBkResultWasOpen As Boolean
PathCrnt = ThisWorkbook.Path

On Error Resume Next
Set WBkMaster = Workbooks(WBkMasterName)
If WBkMaster Is Nothing Then
Set WBkMaster = Workbooks.Open(PathCrnt & "\" & WBkMasterName)
Else
'WBkMaster was open so flag not to close it later:
WBkMasterWasOpen = True
End If
On Error GoTo 0
If WBkMaster Is Nothing Then
MsgBox "I was unable to open workbook '" & WBkMasterName & "'.", vbOKOnly
Exit Sub
Else
With WBkMaster
On Error Resume Next
Set WShtMaster = .Worksheets(WShtMasterName)
On Error GoTo 0
If WShtMaster Is Nothing Then
MsgBox "Workbook '" & WBkMasterName & "' does not contain worksheet '" & WShtMasterName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
End With
End If

On Error Resume Next
Set WBkResult = Workbooks(WBkResultName)
If WBkResult Is Nothing Then
Set WBkResult = Workbooks.Open(PathCrnt & "\" & WBkResultName)
Else
'WBkResult was open so flag not to close it later:
WBkResultWasOpen = True
End If
On Error GoTo 0
If WBkResult Is Nothing Then
MsgBox "I was unable to open workbook '" & WBkResultName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
If Not WBkResultWasOpen Then WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
Exit Sub
Else
With WBkResult
On Error Resume Next
Set WShtResult = .Worksheets(WShtResultName)
On Error GoTo 0
If WShtResult Is Nothing Then
MsgBox "Workbook '" & WBkResultName & "' does not contain worksheet '" & WShtResultName & "'.", vbOKOnly
WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
End With
End If

'your final col/fao/name check
'your copying
'your saving

End Sub


Hey guys!


Thanks for looking at this for me!

I will try this out monday when I am t at the office! Thanks again for the advice, you guys rock :cool:

phesago
05-12-2014, 06:14 AM
Hey guys!


Thanks for looking at this for me!

I will try this out monday when I am t at the office! Thanks again for the advice, you guys rock :cool:


So getting the assignment to contstant not permitted for "ColResultFaoName = "A""


Is this because I didnot mention it earlier in the code? Bare with me while i ask some really stupid questions(this is my first attempt at writing a macro, so I pretty much jumped into the deep ened), but I thought I could assign a column where the results would go? Maybe I am over looking something obvious?


Thanks again guys

p45cal
05-12-2014, 08:02 AM
So getting the assignment to constant not permitted for "ColResultFaoName = "A""
Is this because I did not mention it earlier in the code? Bear with me while I ask some really stupid questions(this is my first attempt at writing a macro, so I pretty much jumped into the deep end), but I thought I could assign a column where the results would go? Maybe I am overlooking something obvious?Yep, it's a constant, which means it doesn't change, you said so in the first line: Const ColResultFaoName As Long = 1 where you assigned it the value 1 and a number (a Long). So there are two things wrong, (a) you're trying to change a constant and (b) you're trying to assign a string to a numeric object.

I'm not able to write/adjust the part of your code I replaced with:
'your final col/fao/name check
'your copying
'your saving
because I don't know what your intention is with various variables and constants; what do you want to go where? As a start it'd be a good idea to answer my previous question: " you have declared a variable ColMasterFaoName as Long but never assign anything to it (it holds 0). You use another undeclared variable called ColFaoName. Are these two meant to be one and the same?"

phesago
05-12-2014, 08:53 AM
They are! I may change a few things up, so that they are less confusing. I can see where my want to simplify things ends up complicating it more.

Basically, i just want to pull 1 column form each workbook in a folder(all sharing the same name<faoname>) and put it into a new workbook book. Basically tracking trends from a report by name. I realize that possibly I may have written too long a code to do something so simple. So, would it be wise to have the columns from results/master/workbook(s) different?

I think I may need to learn this a bit more, as it seems I may be missing some fairly obvious stuff. I actually want to add a second column to this, but I do not want to try it untill I have a better understanding of what Im doing.


Thanks again P45cal, I truly appreciate your input.

p45cal
05-12-2014, 12:58 PM
Away 'til thursday without internet

p45cal
05-20-2014, 07:00 AM
try:
Sub FAOName2()
Const ColResultFaoName = "A"
Const FAOName As String = "FAO_Name"
Const WBkMasterName As String = "mater.data.xls"
Const WBkResultName As String = "results.data.xls"
Const WShtMasterName As String = "Combined"
Const WShtResultName As String = "FAO"
Const ColMasterFaoName = "A"
Dim ColMasterCrnt As Long
Dim CountMasterColFoundCrnt As Long
Dim CountMasterColFoundTotal As Long
Dim InxWbkCrnt As Long
Dim PathCrnt As String
Dim RngResult As Range
Dim RowMasterNext As Long
Dim RowResultLast As Long
Dim TempStg As String
Dim WBkMaster As Workbook
Dim WBkResult As Workbook
Dim WShtMaster As Worksheet
Dim WShtResult As Worksheet
Dim WBkMasterWasOpen As Boolean, WBkResultWasOpen As Boolean
PathCrnt = ThisWorkbook.Path

On Error Resume Next
Set WBkMaster = Workbooks(WBkMasterName)
If WBkMaster Is Nothing Then
Set WBkMaster = Workbooks.Open(PathCrnt & "\" & WBkMasterName)
Else
'WBkMaster was open so flag not to close it later:
WBkMasterWasOpen = True
End If
On Error GoTo 0
If WBkMaster Is Nothing Then
MsgBox "I was unable to open workbook '" & WBkMasterName & "'.", vbOKOnly
Exit Sub
Else
With WBkMaster
On Error Resume Next
Set WShtMaster = .Worksheets(WShtMasterName)
On Error GoTo 0
If WShtMaster Is Nothing Then
MsgBox "Workbook '" & WBkMasterName & "' does not contain worksheet '" & WShtMasterName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
End With
End If

On Error Resume Next
Set WBkResult = Workbooks(WBkResultName)
If WBkResult Is Nothing Then
Set WBkResult = Workbooks.Open(PathCrnt & "\" & WBkResultName)
Else
'WBkResult was open so flag not to close it later:
WBkResultWasOpen = True
End If
On Error GoTo 0
If WBkResult Is Nothing Then
MsgBox "I was unable to open workbook '" & WBkResultName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
If Not WBkResultWasOpen Then WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
Exit Sub
Else
With WBkResult
On Error Resume Next
Set WShtResult = .Worksheets(WShtResultName)
On Error GoTo 0
If WShtResult Is Nothing Then
MsgBox "Workbook '" & WBkResultName & "' does not contain worksheet '" & WShtResultName & "'.", vbOKOnly
WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
Exit Sub
End If
End With
End If
'your final col/fao/name check:
With WShtResult
If .Cells(1, ColResultFaoName).Value <> FAOName Then
MsgBox "Cell " & .Cells(1, ColResultFaoName).Address(0, 0) & " of worksheet '" & WShtResultName & "' of workbook '" & WBkResultName & "' is not '" & FAOName & "'.", vbOKOnly
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
If Not WBkResultWasOpen Then WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
Exit Sub
End If
'your copying:
RowResultLast = .UsedRange.Row + .UsedRange.Rows.Count - 1
Set RngResult = .Range(.Cells(2, ColResultFaoName), .Cells(RowResultLast, ColResultFaoName))
End With
With WShtMaster
RowMasterNext = .UsedRange.Row + .UsedRange.Rows.Count
RngResult.Copy Destination:=.Cells(RowMasterNext, ColMasterFaoName)
End With
'your saving:
WBkMaster.Save
If Not WBkMasterWasOpen Then WBkMaster.Close SaveChanges:=False
Set WBkMaster = Nothing
If Not WBkResultWasOpen Then WBkResult.Close SaveChanges:=False
Set WBkResult = Nothing
End Sub