New to VBA: Code review

[poncho12345678]

New to VBA, the following code works perfectly, however I would like to condense the copy section using a loop, but not sure how to go about it; I'm learning so any other tips on improving it will be well received; researched on here for well over a day, but have ended up confusing myself.

Sub ImportMonthlyData_Click()
ImportMonthlyData.Caption = ThisWorkbook.Sheets("LABELS").Range("E3")
ImportMonthlyData.BackColor = vbGreen
Workbooks.Open "FEB2016.xlsm"
Workbooks("FEB2016.xlsm").Sheets("JP").Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets("JP").Range("C1:FA45")
Workbooks("FEB2016.xlsm").Sheets("LO").Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets("LO").Range("C1:FA45")
Workbooks("FEB2016.xlsm").Sheets("CF").Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets("CF").Range("C1:FA45")
Workbooks("FEB2016.xlsm").Activate
ActiveWorkbook.Close SaveChanges:=False
Workbooks("DASHBOARD.xlsm").Activate
Worksheets("MAIN").Activate
Range("A1").Select
ActiveWorkbook.Save
End Sub

Any help would be appreciated.

 

[makexcel]

New to VBA, the following code works perfectly, however I would like to condense the copy section using a loop, but not sure how to go about it; I'm learning so any other tips on improving it will be well received; researched on here for well over a day, but have ended up confusing myself.
....
Any help would be appreciated.

test well , if I understand correctly , you would like something like this

Sub ImportMonthlyData_Click() Dim x1 As Long, xSheets As Variant
 xSheets = Array("JP", "LO", "CF")
 
 ImportMonthlyData.Caption = ThisWorkbook.Sheets("LABELS").Range("E3")
 ImportMonthlyData.BackColor = vbGreen
 Workbooks.Open "FEB2016.xlsm"
 For x1 = 0 To UBound(xSheets)
   Workbooks("FEB2016.xlsm").Sheets(xSheets(x1)).Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets(xSheets(x1)).Range("C1:FA45")
  'Workbooks("FEB2016.xlsm").Sheets("JP").Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets("JP").Range("C1:FA45")
  'Workbooks("FEB2016.xlsm").Sheets("LO").Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets("LO").Range("C1:FA45")
  'Workbooks("FEB2016.xlsm").Sheets("CF").Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets("CF").Range("C1:FA45")
 Next x1
 Workbooks("FEB2016.xlsm").Activate
 ActiveWorkbook.Close SaveChanges:=False
 Workbooks("DASHBOARD.xlsm").Activate
 Worksheets("MAIN").Activate
 Range("A1").Select
 ActiveWorkbook.Save
End Sub

 

[poncho12345678]

That was quick! Works perfectly, thank you. I have some homework now to find out what UBound does.. thanks again

 

[makexcel]

That was quick! Works perfectly, thank you. I have some homework now to find out what UBound does.. thanks again

Ubound indicates the number of elements within the variable " xSheets "

 

[poncho12345678]

So if I was to declare say:
wb as String
wb = "FEB2016.xlsm"

I could then use:

Workbooks(wb).Sheets(xSheets(x1)).Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets(xSheets(x1)).Range("C1:FA45")

then I'd only have to change one occurrence per new macro?

 

[makexcel]

yes sure , if the destination sheet is always the same , you have to use the same macro to copy you other months . You should see the file structure and data , in order to optimize the macro

Sub ImportMonthlyData_Click() Dim x1 As Long, xSheets As Variant, xFiles As Variant
 xSheets = Array("JP", "LO", "CF")
 xFiles = "FEB2016.xlsm"
 ImportMonthlyData.Caption = ThisWorkbook.Sheets("LABELS").Range("E3")
 ImportMonthlyData.BackColor = vbGreen
 Workbooks.Open xFiles '"FEB2016.xlsm"
 For x1 = 0 To UBound(xSheets)
  Workbooks(xFiles).Sheets(xSheets(x1)).Range("C1:FA45").Copy Workbooks("DASHBOARD.xlsm").Sheets(xSheets(x1)).Range("C1:FA45")
 Next x1
 Workbooks(xFiles).Activate
 ActiveWorkbook.Close SaveChanges:=False
 Workbooks("DASHBOARD.xlsm").Activate
 Worksheets("MAIN").Activate
 Range("A1").Select
 ActiveWorkbook.Save
End Sub

 

[poncho12345678]

Yep, the destination sheet is always the same. So that's even better, thank you so much for all your help.

 

[makexcel]