0

I'm having an issue with the VBA Range.Find method. What the code is doing is looking through all of the worksheets in a workbook, find any matches to data in an array, and change the color of the cell with the same value as that data.

The code works perfect on the first sheet. Then, on the next sheet, it gets hung in an infinite loop. After stepping through the code it seems that Find returns an address that is in Range format ("A2:A2") the first time it is running on this page but then reverts back to Cell format ("A2") after that. It doesn't do this on the first page, just the second one.

I could write some code to check the value returned and trim it down, but I want to fix the problem, not put a patch on it.

Here's the code that breaks:

For x = 1 To UBound(wksSheets)
    For y = 0 To (UBound(findData) - 1)
        With wkb.Worksheets(x)
            Set rng = .Range(DataRange).Find(findData(y), LookIn:=xlValues, LookAt:=xlWhole, MatchCase:=False)
                If Not rng Is Nothing Then
                    StrtAdd = rng.Address
                    Do
                        .Range(rng.Address).Interior.ColorIndex = 3
                        Set rng = .Range(DataRange).FindNext(rng)
                    Loop While Not rng Is Nothing And Not rng.Address = StrtAdd
                End If
        End With
    Next y
Next x

The first time through on the second page the rng.Address is "A2:A2" and gets stored in StrtAdd. Then, when the code hits the .FindNext(rng) rng.Address changes to "A2". Because of this, rng.Address is never equal to StrtAdd even though they are talking about the exact same cell. That's the infinite loop.

Any ideas on the best way to fix this?

wksSheets is an array that contains the worksheet names

findData contains the data that is to be found

Thanks in advance!!

  • I've never heard of `rng.Address` returning something like `A2:A2`. What's the value of `DataRange`? – BigBen Oct 02 '20 at 17:32
  • Only observations: `wksSheets` is not being properly used. You should use `With wkb.Worksheets(wksSheets(x))`. The outer parentheses in `(UBound(findData) - 1)` are redundant. What's hidden in the last element that you don't' want to run through the loop? `.Range(rng.Address)` can be written just like `rng`. Could be more efficient using `Application.Union`. – VBasic2008 Oct 02 '20 at 19:10
  • Can you provide values in the code and example values locations in the sheet, enough that we can reproduce the problem. Currently I cannot. I would also use from lbound to ubound for both arrays when looping. – QHarr Oct 02 '20 at 21:44
  • @BigBen DataRange is a string with value of "A2:D18" – Curt Alford Oct 05 '20 at 14:45
  • @QHarr The data in the range are serial numbers from Dell laptops and MS Surface tablets. They are hard-formatted in the sheets as Text so that no leading zeros truncate. Straight values are being used, no formula. Any flat value will work for testing. – Curt Alford Oct 05 '20 at 14:52
  • @VBasic2008 -I'm not understanding why I should use the format "wkb.Worksheets(wksSheets(x))" instead of simply putting "wkb.wksSheets(x)" as I currently have. Is it for programmatic reasons or aesthetic ones? Does my usage slow down the execution? -Thanks! I'll remove them. -The last element is a worksheet that sums various data from the other sheets in the workbook. It doesn't need to be touched with this set of code. - Ah, ok. I'll try that. - I'm not sure how `Application.Union` would help me here. Could you elaborate? – Curt Alford Oct 05 '20 at 15:05
  • It seems that you know what your doing with `findData`. By using `Application.Union` you could collect the cells into a range and 'color' them in one go (per worksheet). Let's say you have 5 sheets in `wksSheets` array. When you loop `For x = 1 to ub..` then in the first iteration `x=1` then `With wkb.Worksheets(1)`. In the next iteration `x=2` then `With wkb.Worksheets(2)` etc. Instead of e.g. `With wkb.Worksheets("Data")`, `With wkb.Worksheets("Sheet8")` or whatever the names are in `wksSheets` array. It would be better if you could post the complete code since we are missing context here. – VBasic2008 Oct 05 '20 at 15:24
  • @VBasic2008 AH! I see. Decrease the number of sheet interactions and thereby increase the speed. Good idea! – Curt Alford Oct 05 '20 at 17:00

1 Answers1

0

Here is the code I ended up using. I still don't know why sometimes I am getting an address of A2:A2 and sometimes A1 but it does patch the issue.

I used InStr to find the : and then Left to knock the extra off.

I also incorporated the suggestions folks left about cleaning up the code.

For x = 1 To UBound(wksSheets)
        For y = 0 To UBound(findData) - 1
            With wkb.Worksheets(x)
                Set rng = .Range(DataRange).Find(findData(y), LookIn:=xlValues, LookAt:=xlWhole, MatchCase:=False)
                    If Not rng Is Nothing Then
                        z = InStr(rng.Address, ":")
                        If z > 1 Then
                            StrtAdd = Left(rng.Address, (z - 1))
                        Else:
                            StrtAdd = rng.Address
                        End If
                        Do
                            rng.Interior.ColorIndex = 3
                            Set rng = .Range(DataRange).FindNext(rng)
                        Loop While Not rng Is Nothing And Not rng.Address = StrtAdd
                    End If
            End With
        Next y
    Next x

While it's a patch, it's a working patch.

I didn't use @VBasic2008's suggestion of Application.Union because the code currently functions properly and I've got to get a version out. If I run into speed issues I will go back and make a new version.

Thanks everyone.