VBA macro to move data across workbooks

Question

I'm new to VBA and am trying to write a macro to move data from an exported file to an excel template. The code below works, however I have to add many more ranges of data and am worried it will be hefty to run.

Sub MoveData()
 Dim fileName As String
 Sheets("Cover").Select '
 range("B5").Select
 fileName = Selection.Value
 Dim path As String
 path = "C:\Users\(name)\Documents\(folder)\" & fileName & ".csv"
 Dim currentWB As Workbook
 Set currentWB = ThisWorkbook
 Dim openWB As Workbook
 Set openWB = Workbooks.Open(path)
 Dim openWs As Worksheet
 Set openWs = openWB.Sheets(fileName)
 openWs.range("C2:C51").Copy
 openWB.Close (False)
 currentWB.Worksheets("Bms").Activate
 range("C7:C56").Select
 ActiveSheet.Paste
End Sub

I took the base code from here(the closed section) and modified it to fit my needs. The file I am extracting data from will change, but will always be in the same folder, which is why I set up the path as such. I am moving the data from C2 and downwards to C7-down on the template. C56 is currently the last used cell on the template, however that is subject to change so I would prefer to get rid of the lower bound.

If there is a better way to write this, I would love some input. Since I'm still new at this, if you could explain your code as well, that would be very beneficial.

Answer 1

It's a nitpicky thing, but I prefer explicit scoping.

Sub MoveData()

Should be

Public Sub MoveData()

---

There are lots of reasons to avoid Activating and Selecting objects. Performance is just one of them.

Dim fileName As String
 Sheets("Cover").Select '
 range("B5").Select
 fileName = Selection.Value

Would be better written as

Dim fileName As String
fileName = Worksheets("Cover").Range("B5").Value

Note that I used the Worksheets collection instead of Sheets. This is because Sheets can have charts, Worksheets can't.

---

Dim currentWB As Workbook
Set currentWB = ThisWorkbook

Good call and good naming. Well done.

---

It's very slow to open a workbook. Luckily, you might not even have to open the book with the source data. I would look into querying the Excel data source with ADODB instead. Once you have a recordset, you can use the CopyFromRecordset method of Range to paste into the current workbook. It's much faster than loading the data source into Excel.

Here's some untested code to get you started.

Dim cn as ADODB.Connection
Set cn = New ADODB.Connection
With cn
 .Provider = "Microsoft.Jet.OLEDB.4.0"
 .ConnectionString = "Data Source=" & path & ";" & _
"Extended Properties=Excel 14.0;"
 .Open
End With
strQuery = "SELECT * FROM [Sheet1$C2:C51]"
Dim rst As New ADODB.Recordset
rst.Open sqlQuery, cn
currentWB.Worksheets("Bms").Range("C7").CopyFromRecordset rst

---

I don't think you understand what parenthesis do in procedure calls.

openWB.Close (False)

There's no harm in passing a boolean literal ByVal in this case, but make sure you save yourself the headache down the road and go read that StackOverflow Q&A I linked to. Parenthesis change how the argument is passed to a procedure.

• zbrown: Could you elaborate on the idea of using ADO? I've looked through your link and did some googling but it seems a bit above my head.
• RubberDuck: @Aei I added some untested code that should help you get started.
• Comintern: I'd recommend using a .csv specific connection string rather than specifying Excel. I haven't formally benchmarked them, but the first time I switched from Jet/Excel to Jet/Text it was a big enough improvement that I've never looked back.
• zbrown: @RubberDuck I'm having trouble getting rid of an error in the With block. The error occurs in the .Open line and displays "Could not find installable ISAM." It seems to be a generic error but I can't identify what's causing the problem. Also, I should have mentioned this in the original post, but I'm using Excel 2007.
• zbrown: Fixed by using .Provider = "Microsoft.ACE.OLEDB.12.0", and after a minor issue in the rst.Open line, everything works perfectly now. I appreciate all the help.