0

I try to do a macro that can find it some strings in some cells, but I have a error.

My code is:

Sub MACRO()
    bAlerts = Application.DisplayAlerts
    Application.DisplayAlerts = False
    Dim Totalrows As Integer
    Dim Totalcols As Integer
    Dim Valor As String
    For i = 1 To Worksheets.Count
        Totalrows = Worksheets(i).UsedRange.rows.Count
        Totalcols = Worksheets(i).UsedRange.Columns.Count
        For r = 1 To Totalrows
            For c = 1 To Totalcols
                If InStr(Worksheets(i).Cells(r, c), "cadena buscada") > 0 Then
                    MsgBox "Se encontró una coincidencia"
                End If
            Next c
        Next r
    Next i
    Application.DisplayAlerts = bAlerts
End Sub

I have an error in this line:

If InStr(Worksheets(i).Cells(r, c), "cadena buscada") > 0 Then

Anyone knows where is the error ?

Thanks!

Mr.Beto
  • 115
  • 1
  • 4
  • 12
  • 1
    what's the error, specifically? You can make sure i, r, and c have valid values. That'd be a good start. Running your code on my test workbook doesn't throw and errors. – sous2817 Mar 12 '14 at 15:08
  • There is nothing wrong with the code. It might be that the counts are incorrect. FYI, UsedRange is not the best way to find "used range". – Pankaj Jaju Mar 12 '14 at 15:08

3 Answers3

0

my string I want to find, is a formula like this =C:\Gestion...

The most likely issue is that your cell on the sheet contains error, like #N/A or #DIV/0! and so on.

Loop through each cell is unefficient. Use .Find method instead:

Sub MACRO()
    Dim bAlerts
    Dim LastRow As Long
    Dim LastCol As Long
    Dim ws As Worksheet
    Dim rng As Range
    Dim fAddr As String

    bAlerts = Application.DisplayAlerts
    Application.DisplayAlerts = False

    For Each ws In ThisWorkbook.Worksheets
        With ws.UsedRange
            Set rng = .Find(What:="cadena buscada", _
                                        LookIn:=xlFormulas, _
                                        LookAt:=xlPart, _
                                        MatchCase:=False)
            If Not rng Is Nothing Then
                fAddr = rng.Address
                Do
                    MsgBox "Match found in sheet: " & ws.Name & ", cell: " & rng.Address
                    Set rng = .FindNext(rng)
                    If rng Is Nothing Then Exit Do
                Loop While fAddr <> rng.Address
            End If
        End With
    Next ws
    Application.DisplayAlerts = bAlerts
End Sub

I also changed For i = 1 To Worksheets.Count to For Each ws In Worksheets because it's slightly faster.

Dmitry Pavliv
  • 35,333
  • 13
  • 79
  • 80
  • I try this, but I can't find this string "cadena buscada" in my excel. My program don't find it this string – Mr.Beto Mar 12 '14 at 15:22
  • check whether you really have this substring in your sheet. Try to find it manually in sheet using CTRL+F – Dmitry Pavliv Mar 12 '14 at 15:24
  • Simoco, Find is very Inefficient not sure, in this case if it is faster, but, in my tests looping, most of the time is faster...but its a good idea to time it for this case :) – CRondao Mar 12 '14 at 15:24
  • @CRondao, if you have 10 cells in used range, loop may be faster. But in real sheet whith data find is more efficient way – Dmitry Pavliv Mar 12 '14 at 15:28
  • Okey, well, my string I want to find, is a formula like this =C:\Gestion... How I can find this – Mr.Beto Mar 12 '14 at 15:29
  • @simoco, you should backup your saying with facts, speed will depend on many things, range size, number of matches... etc. Its a range A1:Z250 real enough for you? Sure much more than 10 cells. See my answer update and test it, you will be suprised... – CRondao Mar 12 '14 at 18:13
0

Update your column and row count to a more stable extraction set [link] so that it associates with the entire worksheet used range, rather than standardized used range which breaks on blank cells.

Sub MACRO()
    bAlerts = Application.DisplayAlerts
    Application.DisplayAlerts = False
    Dim Totalrows As Integer
    Dim Totalcols As Integer
    Dim Valor As String
    For i = 1 To Worksheets.Count
        Totalrows = Cells.Find("*", [A1], , , xlByRows, xlPrevious).Row
        Totalcols = Cells.Find("*", [A1], , , xlByColumns, xlPrevious).Column
        For r = 1 To Totalrows
            For c = 1 To Totalcols
                If InStr(Worksheets(i).Cells(r, c), "cadena buscada") > 0 Then
                    MsgBox "Se encontró una coincidencia"
                End If
            Next c
        Next r
    Next i
    Application.DisplayAlerts = bAlerts
End Sub
Community
  • 1
  • 1
Rich
  • 4,134
  • 3
  • 26
  • 45
0

See if you have an error in any of the used cells, if so add this to your code:

if not IsError(worksheets(i).cells(r,c)) then
  if InStr....

To look closer to efficiency try this code

Sub MACRO1()
Dim ws As Worksheet
Dim rng As Range
Dim fAddr As String, p As Long
Set ws = Sheet1
    With ws.UsedRange
        Set rng = .Find(What:=18, _
                                    LookIn:=xlFormulas, _
                                    LookAt:=xlPart, _
                                    MatchCase:=False)
        If Not rng Is Nothing Then
            fAddr = rng.Address
            Do
                p = p + 1
                Set rng = .FindNext(rng)
                If rng Is Nothing Then Exit Do
            Loop While fAddr <> rng.Address
        End If
    End With
'MsgBox p
End Sub

Sub macro2()
Dim ws, c, p As Long
Set ws = Sheet1.UsedRange
For Each c In ws
 If c = 18 Then
  p = p + 1
  End If
Next
'MsgBox p
End Sub
Sub macro3()
Dim ws, c, p As Long, v
Dim nr As Long, nc As Long, r As Long
Set ws = Sheet1.UsedRange
nr = ws.Rows.Count
nc = ws.Columns.Count
v = ws
For r = 1 To nr
 For c = 1 To nc
  If v(r, c) = 18 Then
   p = p + 1
  End If
 Next
Next
'MsgBox p
End Sub

Sub TimeIt()
Dim t1 As Single, t2 As Single
Dim p As Integer, t3 As Single
t1 = Timer
For p = 1 To 300
 MACRO1
Next
t1 = Timer - t1
t2 = Timer
For p = 1 To 300
 macro2
Next
t2 = Timer - t2
t3 = Timer
For p = 1 To 300
 macro3
Next
t3 = Timer - t3
MsgBox t1 & Chr(13) & t2 & Chr(13) & t3
End Sub

With a used range about A1:Z250 filled with integer random numbers 1-20 and looking for all the matches of the value 18, in my computer the times were:

macro1 (using Find) 5.76 secs
macro2 (looping with For each) 3.94 secs
macro3 (looping using an array) 0.6 secs

If there is only one match macro1 and macro3 took both abou 0.6 secs

So if we have several possible matches FIND is the most inefficient (almost, in this case, 9 times slower.

CRondao
  • 1,883
  • 2
  • 12
  • 10
  • 1) I said that `.Find` is faster than looping through _cells_ (but not array) 2) if you look at OP's comment below my answer, `my string I want to find, is a formula like this =C:\Gestion...` - so storing range in array and then loop doesn't work, because search should be made _in formulas_ but not in their result values. That's why `.Find` is best suited here – Dmitry Pavliv Mar 12 '14 at 18:19
  • That's no longer what we're talking about, i just said that find is not always faster and it is not, and as you can see, even without arrays, with several matches, looping in cells was faster. It's a shame when someone have problems saying...."Ok I was not completly right" even faced with facts.....but ok, discussion is over... – CRondao Mar 12 '14 at 18:28