1
votes

I've managed to semi-successfully create a macro that copies the contents of a couple of columns in one (closed) workbook to another, already opened workbook.

Problem is that the code below results in the last row always showing N/A for each column... Do you know what the mistake in my code is? I would prefer to fix it than adding another line of code that removes the N/As.

Here's the code (also I noticed that using ActiveWorkbook or ThisWorkbook slows the macro down considerably... turned off auto calc to make it go faster, but please let me know if you have any further suggestions to simplify the code).

Sub DataFromClosedFile()

On Error GoTo ErrHandler

Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual

x is the closed, source workbook y is the current active workbook into which I will paste the data

Dim x As Workbook
Dim y As Workbook

Dim CA_TotalRows As Integer
Dim CA_Count As Integer

I think this next line of code with the 'ThisWorkbook' slows everything down significantly - not sure why, but oh well...

Set y = ThisWorkbook 'Could also have used ActiveWorkbook
Set x = Workbooks.Open("PATH", True, True)

Next, count the number of rows that need to be copied (I don't want to copy the titles, hence why I start from row 2:

CA_TotalRows = x.Worksheets("August_2015_CA").Range("A2:A" & Cells(Rows.Count, "A").End(xlUp).Row).Rows.Count + 1

Also, I only want to copy columns A:B and E:H from the source databook. Hence the repeated formula. Again, any suggestions on how to maximise the efficiency of the code are appreciated!

For CA_Count = 1 To CA_TotalRows
    y.Worksheets("Sheet3").Range("A1:B" & CA_Count).Formula = x.Worksheets("August_2015_CA").Range("A2:B" & CA_Count).Formula
Next CA_Count

For CA_Count = 1 To CA_TotalRows
    y.Worksheets("Sheet3").Range("C1:F" & CA_Count).Formula = x.Worksheets("August_2015_CA").Range("E2:H" & CA_Count).Formula
Next CA_Count

So at some point after that last code is executed, the last row then is copied as a bunch of N/As... for each of the columns. How to avoid that?!

x.Close False
Set x = Nothing

Application.Calculation = xlCalculationAutomatic

ErrHandler:
    Application.EnableEvents = True
    Application.ScreenUpdating = True

End Sub
2

2 Answers

1
votes

The following declarations should be long.

Dim CA_TotalRows As Long
Dim CA_Count As Long

You cannot depend on the workbook opening up to the August_2015_CA worksheet but you are relying it being the ActiveSheet by not specifying the parent of Cells.

with x.Worksheets("August_2015_CA")
    CA_TotalRows = .Range("A2:A" & .Cells(Rows.Count, "A").End(xlUp).Row).Rows.Count + 1
end with

Note .Cells and not Cells. The prefacing . forces the parent to be the worksheet noted in the With ... End With statement. If August_2015_CA was not the active worksheet on open then you were getting the wrong row count.

1
votes

The error is that you start extracting data from row 2 in Worksheet "August_2015_CA" and paste it from row 1 in "Sheet1". Consequently, the last looping refers to empty cells in "August_2015_CA". Moreover, you are pasting the same data again and again, which slows down the code.

Solution:

1) Finding the last row might be a bit easier:

CA_TotalRows = x.Worksheets("August_2015_CA").UsedRange.Rows.Count

assuming that you have data in Row1

2) Why don't you try to paste all the data in one - this would be much faster than looping:

y.Worksheets("Sheet3").Range("A1:B" & CA_TotalRows - 1).Formula = _ 
x.Worksheets("August_2015_CA").Range("A2:B" & CA_TotalRows).Formula

y.Worksheets("Sheet3").Range("C1:F" & CA_TotalRows - 1).Formula = _  
x.Worksheets("August_2015_CA").Range("E2:H" & CA_TotalRows).Formula