Pasting from another workbook

Question

I have a macro comprised of this and similar Subs. They work, but I feel like it's slowing it down. Two workbooks are open, and various ranges are copied from one that is hidden (wbSource) and pasted into the other (mainWB), which is not hidden. There is formatting in all the cells that need to be copied, not just the values. Is there a way to do this better?

Sub CopyData()
Dim mainWB As Workbook
Dim mainWS As Worksheet
Set mainWB = ActiveWorkbook
Set mainWS = mainWB.Sheets(1)
Dim wbSource As Workbook
Set wbSource = Workbooks.Open(Filename:="H:\Formatting.xlsx")
ActiveWindow.Visible = False
wbSource.Sheets(1).Range("A10:CX15").Copy
mainWS.Paste
Range("A1").Select
wbSource.Sheets(1).Range("A25:M26").Copy
mainWS.Paste
mainWS.Range("AF8").Select
wbSource.Sheets(1).Range("D49").Copy
mainWS.Paste
mainWS.Range("AF10").Select
wbSource.Sheets(1).Range("D51").Copy
mainWS.Paste
wbSource.Close
End Sub

Answer 1

Yes, there is a way to do this better.

Always Option Explicit.

No need (almost always) for .Select, this is a human activity and explicitly referencing the ranges in VBA is more efficient. This also creates extra work for the VBA engine.

You hide your active window, but you do not unhide it.

Don't be afraid to use you favourite search engine to look up even the simplest commands, especially if you are fixing up code generated by the macro recorder. Sometimes these commands have methods and properties that make life easier (e.g. see what I have done with the copy method - look up the "Excel Range.Copy" method to see what gems are there).

However, with the amount of code you have here, I am really surprised that you would notice any slowing down!

Sub CopyData()
 Dim mainWB As Workbook
 Dim mainWS As Worksheet
 Set mainWB = ActiveWorkbook
 Set mainWS = mainWB.Sheets(1)
 Dim wbSource As Workbook
 Set wbSource = Workbooks.Open(Filename:="H:\Formatting.xlsx")
 'ActiveWindow.Visible = False ' Why?
 wbSource.Sheets(1).Range("A10:CX15").Copy mainWS.Range("A10:CX15")
 wbSource.Sheets(1).Range("A25:M26").Copy mainWS.Range("A1") 'Haven't tested this line to see what effect the different range size has.
 wbSource.Sheets(1).Range("D49").Copy mainWS.Range("AF8")
 wbSource.Sheets(1).Range("D51").Copy mainWS.Range("AF10")
 wbSource.Close
End Sub

And the usual staples of setting Application.ScreenUpdating, Application.EnableEvents, Application.Calculation will assist in managing what happens or is shown when the routine is run. ActiveWindow.Visible could be handy, but remember to turn it back on after doing the work. Your ActiveWindow may not be what you think it is.

• Robby: There's LOTS of code happening before and after this bit of code runs in one workbook. This Sub opens Formatting.xlsx, making it visible, but it needs to be not be visible. I just need it open long enough to copy and paste over to the original workbook, and then close.
• Raystafarian: @Robby if you have more, feel free to post it as another question
• Robby: A lot of the other code is written similarly. I haven't had time to try this technique yet, but the plan is to make similar changes to the other code that I have, and hopefully it does make an impact on the load my macro takes on.
• AJD: @Robby: clearly there is more to this than you have exposed. I suspect that refactoring at a higher level would help - but we can only work with what we have given. This single macro will not cause noticeable slowness - the biggest time driver would be the opening and closing of the workbook or the handling of events that we don't know about here. However, if you are opening multiple workbooks (or, even worse, opening the same workbook multiple times) this will all add up. [...]
• AJD: @Robby: [...] So, taking a step back - what problem are you trying to solve, what outcome are you trying to achieve, and is refactoring a single macro going to help or should you rethink your suite of macros?