1

I am writing a code to retrieve a specific date in a (somewhat) large excel spreadsheet(2,000 entries). I just realize that my code will not work and it will only get worse. Could you please advise me.

I give to my function:

  • array() that contain my data from an excel spreadsheet
  • FirstDate which is the date I am looking for, as dd mm yyyy
  • DateSave() to save all the position where this date appears (multiple transaction on the same day)

The code will not be able to work with a database of 5,000 row as it will have to stack it if the date is at the end of the table. What could I do to fix this issue?

Thank you very much

Function looping(array() As Variant, FirstDate As Date, DateSave() As Long)

    Dim i As Long
    Dim PositionInArray As Long

    PositionInArray = 0
    
    For i = LBound(array, 1) To UBound(array, 1)
        
                If array(i, 1) = FirstDate Then

                    ReDim Preserve DateSave(PositionInArray)
                    DateSave(PositionInArray) = i
                    PositionInArray = PositionInArray + 1

                End If
            
                'If end of list and array not initialize ie. Not value in it
                If i = UBound(array, 1) And (Not DateSave) = -1 Then

                    Call looping(array(), FirstDate + 1, DateSave())

                ElseIf i = UBound(array, 1) Then

                    'Array has been initialized
                    Exit For

                End If

            Next i

End Function

Edit: Change data base to excel spreadsheet

Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
VI55
  • 69
  • 8
  • 1
    Are you actually getting an error or is this all theoretical? – Nick.Mc Jan 12 '21 at 09:35
  • Avoid `ReDim Preserve` this comes with extremly high costs. – Pᴇʜ Jan 12 '21 at 09:35
  • If this is actual data in a database, I suggest you write a SQL query to do it instead. – Nick.Mc Jan 12 '21 at 09:36
  • 1
    If this is not data in a database I recomment to use a real database like SQL for your data. Excel is no database! – Pᴇʜ Jan 12 '21 at 09:36
  • Yes actually, it is not in a database, it is in excel, I am going to edit it. Why Redim Preserve is bad? I had the error once. – VI55 Jan 12 '21 at 09:40
  • @VI55 because it needs to rewrite the array everytime you call it. Think of it like creating a new array with the new size, copy each value to the new array, replace the old array with the new array. Very ineffective, and slows down a lot if used a lot of times. – Pᴇʜ Jan 12 '21 at 09:43
  • 1
    Even if it's not data in a database, I would [recommend using SQL](https://stackoverflow.com/a/32478268/111794), as most performant and declarative (which for VBA admittedly isn't saying very much). – Zev Spitz Jan 12 '21 at 09:44

1 Answers1

3

I've renamed the function and parameters. The function returns the result rather than having a ByRef parameter. I've used a collection to store the row indexes.

Function GetDatePositions(ByRef database() As Variant, ByVal searchDate As Date) As Long()
    Const colDates As Long = 1 'the index of the column holding dates
    Dim i As Long
    Dim collRowIndexes As New Collection
    
    For i = LBound(database, 1) To UBound(database, 1)
        If database(i, colDates) = searchDate Then
            collRowIndexes.Add i
        End If
    Next i
    
    If collRowIndexes.Count = 0 Then
        GetDatePositions = GetDatePositions(database, searchDate + 1)
        Exit Function
    End If
    
    Dim res() As Long
    ReDim res(0 To collRowIndexes.Count - 1)
    Dim v As Variant
    
    i = 0
    For Each v In collRowIndexes
        res(i) = v
        i = i + 1
    Next v
    
    GetDatePositions = res
End Function

EDIT

There is no need to search each consecutive date. We just need to keep track of the next date that is bigger than the search date.

Function GetDatePositions(ByRef database() As Variant, ByVal searchDate As Date) As Long()
    Const colDates As Long = 1 'the index of the column holding dates
    Dim i As Long
    Dim collRowIndexes As New Collection
    Dim dateFound As Boolean
    Dim nextDate As Date
    Dim tempDate As Date
    
    dateFound = False
    For i = LBound(database, 1) To UBound(database, 1)
        tempDate = database(i, colDates)
        If tempDate = searchDate Then
            dateFound = True
            collRowIndexes.Add i
        Else
            If Not dateFound Then
                If searchDate < tempDate Then
                    If nextDate = 0 Then
                        nextDate = tempDate
                    ElseIf tempDate < nextDate Then
                        nextDate = tempDate
                    End If
                End If
            End If
        End If
    Next i
    '
    If collRowIndexes.Count = 0 Then
        If nextDate = 0 Then
            Err.Raise 5, "GetDatePositions", "No date found"
        Else
            GetDatePositions = GetDatePositions(database, nextDate)
            Exit Function
        End If
    End If
    
    Dim res() As Long
    ReDim res(0 To collRowIndexes.Count - 1)
    Dim v As Variant
    
    i = 0
    For Each v In collRowIndexes
        res(i) = v
        i = i + 1
    Next v
    
    GetDatePositions = res
End Function

Obviously, an assumption that all dates are rounded is made. But if dates also contain time (hours, minutes, seconds) then tempDate = database(i, colDates) needs to be replaced with tempDate = VBA.Int(database(i, colDates))

Cristian Buse
  • 4,020
  • 1
  • 13
  • 34
  • 1
    You could remove the recursive step entirely and transform it into a `Do Until` loop. – Tomalak Jan 12 '21 at 09:46
  • @Tomalak Yes, that would save the creation of a new stack frame each time the function is called. I just made a quick answer that would not confuse the OP too much – Cristian Buse Jan 12 '21 at 09:49
  • 2
    Hm, I think removing recursion would actually result in a less confusing solution, especially for a beginner. Also it would literally never run out of stack space. – Tomalak Jan 12 '21 at 09:52
  • @Tomalak Thanks for the suggestion. A good improvement would be to keep track of the next date. There is no need to increment by +1. I will edit my response soon – Cristian Buse Jan 12 '21 at 09:55
  • That is some amazing code, thank you so much. I feel like a total beginner when reading this... I know it is going to sound stupid, but how do I access the value stored into res in the sub calling this function? – VI55 Jan 12 '21 at 10:04
  • 1
    @VI55 You would do something like ```x = GetDatePositions(db, myDate)``` and you can then use ```x``` as you would have used ```DateSave```. Alternatively, you could return the Collection instead of transforming to an array of Long – Cristian Buse Jan 12 '21 at 10:08
  • @CristianBuse , and the last line : GetDatePositions = res Return the whole array, then I can access the values like so ? `Lbound(res,1)` – VI55 Jan 12 '21 at 10:11
  • 1
    @VI55 If you read the above reply, where I called it ```x``` then yes, you could do ```LBound(x,1)``` if you need to. Anyway, I am just editing the answer with a bigger improvement. Just refresh the page in a couple of seconds – Cristian Buse Jan 12 '21 at 10:13
  • @Tomalak The LBound and UBound work with the index of the dimension. In our case we are looking at a 2D array and hence 1 stands for rows and 2 for columns. We use 1 here – Cristian Buse Jan 12 '21 at 10:31
  • @Tomalak I laughed so hard at this. Didn't know the expression. I had one yesterday :) – Cristian Buse Jan 12 '21 at 11:11
  • It happens. I don't have MS Office handy atm so I had no way of testing. – Tomalak Jan 12 '21 at 13:21