3

I've set up some VBA code in Excel that asks the user to select a second worksheet, then searches it for a value (a shared key linking the two sets of data, found 6 columns after Rng, where I want to add the retrieved value) in the second table and adds a value from that row to a column in the original table. The part of the program that I would like to adjust is the loop below.

It works fine if when I leave in the line to activate the CurFile workbook. But it means my screen is flashing a lot back and forth between the two workbooks. And once I start getting into hundreds or thousands of lines of data it will be ridiculously slow.

When I comment out that line, the value for FindCID doesn't change and it seems to just keep on refilling the same line, even though the value for r is updating. If after a few loops I add the activate line back in, it resumes properly filling in the results several lines down.

How can I streamline this? I originally was using ThisWorkbook references but even with explicitly defining CurFile (CurFile = ActiveWorkbook.Name) earlier it doesn't seem to go back to that workbook to look up the next value to search for, unless I reactivate the sheet.

Do While r <= maxRows

With Workbooks(CurFile).Worksheets("Sheet1")
    Set Rng = .Range(Cells(r, c), Cells(r, c))
End With

FindCID = Rng.Offset(0, 6).Value

If Trim(FindCID) <> "" Then
    With Workbooks(FN)   ' found earlier by a function
       .Activate
    End With

    With Sheets("Sheet1").Range("D:D")
        Set FoundCell = .Find(What:=FindCID)
            If Not FoundCell Is Nothing Then
                PathLen = FoundCell.Offset(0, 2).Value
  Workbooks(CurFile).Sheets("Sheet1").Activate 'If I comment out this line it doesn't work
                Rng.Value = PathLen
                MsgBox "CID found in " & FoundCell.Address & " Its value is " & PathLen
            Else
                MsgBox "Nothing found"
            End If
    End With
End If

On Error Resume Next

r = r + 1
Loop
pnbjam
  • 33
  • 4
  • 4
    Set workbooks. You know how to set ranges. It's the same idea. – findwindow Sep 25 '15 at 18:37
  • 3
    change `Set Rng = .Range(Cells(r, c), Cells(r, c))` tp `Set Rng = .Range(.Cells(r, c), .Cells(r, c))` – Scott Craner Sep 25 '15 at 18:38
  • 1
    See the answer here. http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros – MatthewD Sep 25 '15 at 18:46
  • Thanks Scott Craner - Can't say I understand why that slight change works (I couldn't even see the difference the first few times I looked!) but it works perfectly. – pnbjam Sep 25 '15 at 19:16
  • Still a little confused about why I would need to Set a workbook and/or worksheet when I already defined it as part of the Rng range setting. I could be missing something in MatthewD's reference but I can't wrap my head around it right now. Scott Cranner's solution did the trick without having to Set anything else! – pnbjam Sep 25 '15 at 19:22
  • 2
    `set` and `with` are different. What Scott did was reference the workbook(curfile) and "sheet1" via the with block. `cell` refers to activesheet. `.cell` refers to the sheet indicated by the `with` statement. That period makes all the difference ^_^ – findwindow Sep 25 '15 at 19:49
  • 1
    Also, if you're looking to avoid screen flashes and also speed up the loop iteration, try turning off screen updating at the beginning of your code, than turning it back on at the end. I.e.: `Application.ScreenUpdating=False` – CBRF23 Sep 25 '15 at 21:08

1 Answers1

1

Actually when working with objects, in most of the cases, there is no need to activate the workbooks\worksheets. This is your code with some modifications in this regard:

Application.ScreenUpdating = False '(as suggested by CBRF23)
'......
'begining of your code
'......

Do While r <= maxRows

    With Workbooks(CurFile).Worksheets("Sheet1")
        Set Rng = .Cells(r, c) '(1)
    End With

    FindCID = Rng.Offset(0, 6).Value2        
    If Trim(FindCID) <> "" Then
        Set FoundCell = Workbooks(FN).Sheets("Sheet1").Range("D:D").Find(What:=FindCID)
        If Not FoundCell Is Nothing Then Rng.Value = FoundCell.Offset(0, 2).Value2
    End If

    r = r + 1
Loop
'......
'rest of your code
'......
Application.ScreenUpdating = True

(1) Notice that way the Range is defined as it’s made of only once Cell; but if the range has more than one Cell i.e. from Cell(r,c) to Cell(r,c+5) then you need to use the form:

Set Rng = Range(.Cells(r, c), .Cells(r, c+5))

There is no need to add a period . before Range as the range is defined by the Cells within the Range command. By using the period . before the Cell command they are referred as part of the

With Workbooks(CurFile).Worksheets("Sheet1")

However if the Range is defined as A1:F1 then the period . has to be added before the Range as in:

Set Rng = .Range(“A1:F1”)

I removed the MsgBox commands as I believe they were just for testing purposes. Not really showing these messages for hundreds or thousands lines of data. Isn’t it?

EEM
  • 6,601
  • 2
  • 18
  • 33
  • Thanks for this additional work and explanation! I didn't think it should be necessary to activate a sheet to search it, but somehow as I was cobbling the code together that's how I got it to work. (Perhaps that was before I defined the workbooks specifically as CurFile and FN and/or got the references right.) I'm a lot clearer now on referencing workbooks, sheets, ranges and cells - I've just been piecing this together based on examples I've found without always understanding exactly why it works, though being able to grasp the gist of it. And I looked up info on Value2 - Interesting! – pnbjam Sep 29 '15 at 13:16
  • Tested this and after removing the End With: just above the r = r + 1 it runs, and quickly too! (And indeed, my MsgBox lines were just for testing; I mostly commented them out unless I was stepping through things in the debugger. Otherwise it's very slow when you have to hit OK that many times!) Thanks again EEM, this was just what I needed. – pnbjam Sep 29 '15 at 13:25
  • Glad it solved your problem. Thanks for pointing the typo for the end with, somehow I did not clear it after moving it above. Answer adjusted.. – EEM Sep 29 '15 at 15:33
  • For testing and debugging you can print the values you want to check _(variables, addresses of ranges, etc)_ in the immediate window using the `Debug.Print` command – EEM Sep 29 '15 at 15:39