Merging Data From Workbooks macro

[phesago]

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]

"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]

"On Error Resume Next" should appear 1st and never again

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

[phesago]

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

[phesago]

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

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]

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]

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]

Away 'til thursday without internet

[p45cal]

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