vba Code Performance

[Klips]

Hi all,

I am experiencing a performance issue with my vba code. I think that
I'm making a mistake somewhere since a similar code this used to run
alot faster. Here are the details:

- I have tested the code on both Excel 2002 and 2003
- The code that seems to be causing the problem is this:

Sub importItems(ByRef sourceRange as Range, ByRef destinRange as
Range, ByRef sourceWB As Workbook, ByRef rowTotal As Long)

On Error GoTo ErrHandler:

Dim dataRow as Range
Dim rowCount as Long
Dim readRows as Long
Dim totalRows as Long
rowCount = 0
readRows = 0
totalRows = sourceRange.Rows.Count
For Each dataRow in sourceRange.Rows
If dataRow.Cells(1, 2) = "Yes" Then
With dataRow
rowCount = rowCount + 1
destinRange.Cells(rowCount, 1) = .Cells(1, 1)
destinRange.Cells(rowCount, 2) = .Cells(1, 9)
destinRange.Cells(rowCount, 8) = WorksheetFunction.VLookup(.Cells(1,
14), sourceWB.Sheets("Releases").Range("Data"), 2, False)
destinRange.Cells(rowCount, 9) = .Cells(1, 7)
End With
End If
readRows = readRows + 1
frmImport.prg_import.Value = (readRows / totalRows) * 100
Next dataRow
rowTotal = rowCount
Exit Sub

ErrHandler:
MsgBox "An error occured: " & Err.Description & " (" & Err.Number &
")", vbCritical, "Temp Data Procedure Error"

End Sub

- The code is intended to import some data from another excel file.
The amount of data that is transfered is about 13 x 1600.

The function works, but it is extremely slow. I had pretty much the
same code inside my calling function and it was a lot faster. Once I
moved the code into a seperate procedure, the performance took a big
hit. I'm not sure what could be the problem. If anyone wants to see
the complete file, I'll be happy to provide it.

Thanks in advance for all help

 

[Carl Hartness]

No one seems to have responded yesterday, so I'll take a shot.
You said this ran faster when it was in your calling function. Why
not leave it there?
I'm suspicious of
For Each dataRow In sourceRange.Rows
I'm not sure how VBA handles this. If dataRow is a pointer into the
original data, it
won't be so slow, but it is probably a separate object with a lot of
columns to populate which will never be used.

Referencing individual cells is slow. Working in memory arrays is
much faster. Pull sourceRange into an array, copy the needed values
to a second array, and put the second range to destinRange.

Your code doesn't populate continuous columns in destinRange. If the
columns in between have other data, then two output arrays will be
needed.

If you haven't worked with arrays before, here's a starter snippet:
Dim sAry As Variant, dAry As Variant, x As Long
' populate sAry from sourceRange
sAry = sourceRange
ReDim dAry(1 To UBound(sAry, 1), 1 To 9)

For x = 1 To UBound(sAry, 1) ' sAry row count
If sAry(x, 2) = "Yes" Then
rowCount = rowCount + 1
dAry(rowCount, 1) = sAry(x, 1)
dAry(rowCount, 1) = sAry(x, 1)
dAry(rowCount, 2) = sAry(x, 9)
dAry(rowCount, 8) = WorksheetFunction.VLookup(sAry(x, 14),
_
sourceWB.Sheets("Releases").Range("Data"), 2, False)
dAry(rowCount, 9) = sAry(x, 7)
End If
Next x

' put dAry to destinRange
' WARNING: overwrites data already there, doesn't change cell
format.
Range(destinRange.Cells(1, 1), _
destinRange.Cells(rowCount, 9)) = dAry

vlookup to sourceWB might be a slow step. The first two columns of
sourceWB.Sheets("Releases").Range("Data") can be pulled into an array
and searched with a loop.

Carl.