Workbooks.Open Range("M1").Value & ".xlsx"
Workbooks.Open
is a function - it returns a value - in this case, a reference to the Workbook
object that was opened, if the function return successfully without raising any errors.
You are discarding that object reference. Use it!
Dim book As Workbook
Set book = Workbooks.Open(ActiveSheet.Range("M1").Value & ".xlsx")
This instruction assumes opening the workbook made it the ActiveWorkbook
:
Sheets("BBG COMDTY - Cur Data").Select
Don't work off side-effects and assumptions.
Dim currentCOMDTYSheet As Worksheet
Set currentCOMDTYSheet = book.Worksheets("BBG COMDTY - Cur Data")
You don't need to Select
a Range
to Copy
it. You only need to Select
a Range
when you want to work off the Selection
, and you almost never want to do that.
Range("A3:AF10000").Select
Selection.Copy
Instead, just work off the Worksheet
object you've got:
currentCOMDTYSheet.Range("A3:AF10000").Copy
But let's hold off a second, and see where that's being copied to first:
Windows("EDM Summary Macro 1.1.xlsm").Activate
Sheets("BBG COMDTY - Prev Data").Select
Range("A3").Select
Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
:=False, Transpose:=False
(side note: line continuations _
should generally be avoided, but when you do use them, avoid splitting a named argument in the middle - SkipBlanks:=False
should be on a single line)
I'm assuming the "EDM Summary Macro 1.1.xlsm" workbook is the workbook that's hosting your VBA project. If that's the case, then you don't need to dereference its Window
off the Windows
collection, nor to dereference it off some Workbooks
collection (which would be a much more straightforward way to activate that workbook than getting its Window
). The workbook that's hosting your VBA project is always in scope, and it's always called ThisWorkbook
(unless you renamed it, but don't do that).
And since all worksheets in ThisWorkbook
that exist at compile-time each have a code name, you don't need to dereference the worksheet at all - it's right there, readily usable. Maybe it's called Sheet3
or Sheet21
though: find the "BBG COMDTY - Prev Data" sheet in the VBE's Project Explorer, then set its (Name)
property to a valid VBA identifier, e.g. PrevCOMDTYDataSheet
. And now we can paste directly there:
currentCMDTYSheet.Range("A3:AF10000").Copy PrevCOMDTYDataSheet.Range("A3")
But that would bring in formats and borders and validations and formulas, and we only want the values. In that case, we don't need to even get the clipboard involved:
PrevCOMDTYDataSheet.Range("A3:AF10000").Value = currentDataSheet.Range("A3:AF10000").Value
And done!
Similar here:
Windows("EDM_Matcher_Inbox_20191202.xlsx").Activate
Sheets("BBG PK BBGID - Cur Data").Select
Range("A3:AJ10000").Select
Application.CutCopyMode = False
Selection.Copy
Windows("EDM Summary Macro 1.1.xlsm").Activate
ActiveWindow.ScrollWorkbookTabs Sheets:=1
ActiveWindow.ScrollWorkbookTabs Sheets:=1
Sheets("BBG PK BBGID - Prev Data").Select
Range("A3").Select
Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
:=False, Transpose:=False
You already have the book
reference, so no need to activate the window again; the BBGID data sheet needs an object reference:
Dim currentBBGIDSheet As Worksheet
currentBBGIDSheet = book.Worksheets("BBG PK BBGID - Cur Data")
And then similar as above:
PrevBBGIDSheet.Range("A3:AJ10000").Value = currentBBGIDSheet.Range("A3:AJ10000").Value
Same with the last block:
ActiveWorkbook.Activate
Windows("EDM_Matcher_Inbox_20191202.xlsx").Activate
Sheets("BBG BO - Cur Data").Select
Range("A3:AH10000").Select
Application.CutCopyMode = False
Selection.Copy
Windows("EDM Summary Macro 1.1.xlsm").Activate
ActiveWindow.ScrollWorkbookTabs Sheets:=2
Sheets("BBG BO - Prev Data").Select
Dim currentBBGBOSheet As Worksheet
Set currentBBGBOSheet = book.Worksheets("BBG BO - Cur Data")
PrevBBGBOSheet.Range("A3:AH10000").Value = currentBBGBOSheet.Range("A3:AH10000").Value
So, recap: first step is to name your sheets in ThisWorkbook
. You want PrevCOMDTYSheet
, PrevBBGIDSheet
, and PrevBBGBOSheet
worksheets named in the VBA project, and then this can be a first-pass cleanup:
Public Sub OpenPrev()
Dim book As Workbook
Set book = Workbooks.Open(ActiveSheet.Range("M1").Value & ".xlsx")
Dim currentCOMDTYSheet As Worksheet
Set currentCOMDTYSheet = book.Worksheets("BBG COMDTY - Cur Data")
PrevCOMDTYSheet.Range("A3:AF10000").Value = currentCOMDTYSheet.Range("A3:AF10000").Value
Dim currentBBGIDSheet As Worksheet
currentBBGIDSheet = book.Worksheets("BBG PK BBGID - Cur Data")
PrevBBGIDSheet.Range("A3:AJ10000").Value = currentBBGIDSheet.Range("A3:AJ10000").Value
Dim currentBBGBOSheet As Worksheet
Set currentBBGBOSheet = book.Worksheets("BBG BO - Cur Data")
PrevBBGBOSheet.Range("A3:AH10000").Value = currentBBGBOSheet.Range("A3:AH10000").Value
End Sub
Second-pass, I'd remove the hard-coded 10000 row numbers, and actually work out what actual size Range
needs to be copied over.