0

I need your help. I have this main macro that opens a previous day file by using a cell reference (location of the file and file name). The opened file is used to copy the data from its worksheets (4 worksheets)to the main macro. I recorded the macro and the date was hard coded. When i will run the macro for next week, it will give me an error since the previous file name was hardcoded. I tried using activeworkbook but it wont work since i go back and fort between the main file and preivous file when copy pasting.

sample query below

Sub OpenPrev()
'
' OpenPrev Macro
'

'
    ChDir "\\hfx1nas02\hfx1_data02_sh$\Share2\HFX MDBG\Marlon\MarkitEDM\EDM_WORK"
    Workbooks.Open Range("M1").Value & ".xlsx"
    Sheets("BBG COMDTY - Cur Data").Select
    Range("A3:AF10000").Select
    Selection.Copy
    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

    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

    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
BigBen
  • 46,229
  • 7
  • 24
  • 40
Jun
  • 1
  • 2
    See [How to Avoid Using Select](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba) first. – BigBen Dec 11 '19 at 18:14
  • 1
    As far as your main issue, `Dim wb as Workbook`, then `Set wb = Workbooks.Open(Range("M1").Value & ".xlsx")`. Now you have a reference to the opened workbook and you don't have to refer to it by name. – BigBen Dec 11 '19 at 18:15
  • Rule #1: never use macro recorder code as-is. You hardly ever **need** to `Select` or `Activate` anything to do 99.99999% of everything you could ever want to do in VBA. – Mathieu Guindon Dec 11 '19 at 18:34

1 Answers1

1
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.

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
  • wow thank you so much Mathieu! It is working now and i have learned a lot from this! for the rage like Range("A3:AJ10000") I put 10000 coz the output data i am copying varies daily, how can set a range whereby it will just copy all the data in that range – Jun Dec 12 '19 at 19:38