VBA Macro to find rows and store in array runs slow

Question

So I made a simple VBA macro to run over ~7000 rows of data - the idea is that .Find finds the cells which contain "1" in column G, so that it can store the row number in an array which I shall later throw back to another sub

Unfortunately the code takes too long to run - it begs the question, have I created an infinite loop in my code? Or is asking it to loop a .find operation over 7000 cells too much for vba to handle at a reasonable speed? (i.e. do I need to improve efficiency in areas?)

Option Explicit

Public Sub splittest()

Dim sheet As Object, wb As Workbook
Dim rangeofvals As Range
Dim pullrange As Variant
Dim c As Long
Dim dynarr() As Variant

Dim xvalue As Long
Dim firstaddress As Variant
Dim count As Long
Set wb = ThisWorkbook
Set sheet = wb.Sheets("imported d")
Set rangeofvals = Range("G1:G6939")
'need to set pull range at some later point

Call OptimizeCode_Begin 'sub which turns off processes like application screen updating

xvalue = 1
ReDim dynarr(3477) 'hardcoded, replace with a countif function at some point
count = 0

With wb.Sheets("imported d").Range("G1:G6939")
     c = rangeofvals.Find(xvalue, LookIn:=xlFormulas).Row
    If c >= 0 Then
        dynarr(count) = c
       ' MsgBox firstaddress
        Do
          '  MsgBox c
            c = rangeofvals.FindNext(Cells(c, 7)).Row
            dynarr(count) = c 'apparently redim preserve would be slower
         Loop While c >= 0
        End If


Call OptimizeCode_End 'sub which turns back on processes switched off before


End With
End Sub

Answer 1

If you know the column is G and all the data is contiguous, then just loop through the rows and check the cell value directly:

Dim rows As New Collection
Dim sheet As Worksheet 
Dim lastRow, i As Integer

Set sheet = ThisWorkbook.Sheets("imported d")
lastRow = sheet.Cells(1,7).CurrentRegion.Rows.Count

For i = 1 to lastRow
    If (sheet.Cells(i,7).Value = 1) Then
        rows.Add i
    End If
Next

Unclear how the data is being used in the other sub but collection is definitely more efficient storage object for adding iteratively when the total item count is indeterminate. If you want to convert to an array then you can do so efficiently afterward since the collection tells you the item count. I'm not really sure why you would need an array specifically but I'm not going to tell you not to without seeing the client sub. Also note that the declaration of sheet was changed from Object to Worksheet since it is better to use the specific data type if possible.

Answer 2

...yep it was an infinite loop.

the line:

Loop While c >= 0

caused it as there is never an occasion c is less than 0 - back to the drawing board for me!