3
\$\begingroup\$

I would like to speed up this piece of code. The macro gets a search variable from the worksheet "RAW_DATA_ARCHIVE" and then it should find it in the worksheet "RAW DATA". When it finds accordance then the value of column A from worksheet "RAW DATA" will be written in the last column of "RAW_DATA_ARCHIVE".

 For i = 2 To ws1LRow
    SearchString = ws1.Range("A" & i).Value
    Set aCell = ws2.Columns(5).Find(What:=SearchString, LookIn:=xlValues, _
                LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext, _
                MatchCase:=False, SearchFormat:=False)
    'If match found
    If Not aCell Is Nothing Then
        ws1.Cells(i, 80).Value = ws2.Cells(aCell.Row, 1).Value

    End If
Next
\$\endgroup\$
  • 1
    \$\begingroup\$ You don't need a VBA macro to do this. Have you seen what VLOOKUP can do for you? Try something like this in column 80 of sheet Raw_Data_Archive: =IFERROR(VLOOKUP(A2,'RAW DATA'!E:E,1,FALSE),"") \$\endgroup\$ – Mathieu Guindon Apr 10 '15 at 13:30
4
\$\begingroup\$

If I understand correctly what you're doing, then VBA is simply not the right tool for this task. Excel has a built-in VLOOKUP function specifically for this purpose - try something like this in column 80 of the RAW_DATA_ARCHIVE sheet:

=IFERROR(VLOOKUP(A2,'RAW DATA'!E:E,1,FALSE),"")

That said, you did post some VBA code, albeit just a loop. Here are some thoughts, mostly on naming style:

  • ws1 and ws2 would probably be better off as sourceSheet and targetSheet.
  • ws1LRow would be more meaningful as simply lastRow
  • i is usually fine as a loop counter, but here it has a clear meaning: targetRow
  • aCell is really a searchResult

Using such meaningful names instantly makes it easier to read the code, which makes it easier to maintain/modify without breaking anything.

It also eliminates the need for explanatory comments:

'If match found
If Not aCell Is Nothing Then

vs.

If Not searchResult Is Nothing Then

A number of things explain why your code is slow.

  1. You're using procedural code instead of built-in worksheet functions
  2. You're writing to cells in a loop, presumably with Application.ScreenUpdating = True and Application.Calculation = xlCalculationAutomatic, which means many things happen every time you write a value into a cell.
\$\endgroup\$
  • \$\begingroup\$ Firstly the target sheet get replaced every week, so i need vba. Besides the "vloockup" doesn't help me, because my target value is on the left side of the search value. \$\endgroup\$ – NewOne Apr 14 '15 at 10:17
  • \$\begingroup\$ Had you answered to my comment 4 days ago my answer might have been different. I see you're using index+match here.... well, use that instead of a vlookup and you'll save yourself 10 minutes of looping.WorksheetFunction is calling, well, the worksheet functions after all - only much, much slower. \$\endgroup\$ – Mathieu Guindon Apr 14 '15 at 11:39
0
\$\begingroup\$

To solve this problem I found the "Index-Match" functions. It is a little bit faster but also take over 10 minutes for about 160000 Cells. Meanwhile, I think it's too much data to get it faster.

With Application.WorksheetFunction
    On Error Resume Next
    For Each searchCell In searchRange
        targetSheet.Cells(searchCell.Row, 80) = .Index(sourceSheet.Columns(1), .Match(searchCell.Value, sourceSheet.Columns(2), 0))
    Next
End With
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.