Collection slow

[tiredofit]

I have some data (spanning a few hundred thousand rows) in column A, such as:

Day
Monday
Tuesday
Friday

ClsData:

Option Explicit
    Private pDayOfWeek As String
    Private pFruit As String
Public Property Get DayOfWeek() As String
    
    DayOfWeek = pDayOfWeek
    
End Property
Public Property Let DayOfWeek(ByVal D As String)
    
    pDayOfWeek = D
    
End Property
Public Property Get Fruit() As String
    
   Fruit = pDayOfWeek
    
End Property
Public Property Let Fruit(ByVal F As String)
    
    pDayOfWeek = F
    
End Property

Standard Module:

Option Explicit
Sub Test()
    Dim DataArray() As Variant
    
    DataArray = Sheet1.Cells(1, 1).CurrentRegion.Value
    
    Dim DataArrayRows As Long
    
    DataArrayRows = UBound(DataArray(), 1)
    
    Dim FruitArray() As Variant
    
    ReDim FruitArray(1 To DataArrayRows, 1 To 1) As Variant
    
    Dim Counter As Long
    
    Dim MyData As ClsData
    
    Dim MyColl As Collection
    Set MyColl = New Collection
    
    For Counter = 2 To DataArrayRows
            
        Set MyData = New ClsData
        
        MyData.DayOfWeek = DataArray(Counter, 1)
        
        Select Case MyData.DayOfWeek
        
            Case "Monday"
                
                MyData.Fruit = "Orange"
                
            Case "Tuesday"
                
                MyData.Fruit = "Apple"
            
            Case Else
            
                MyData.Fruit = "Banana"
                
        End Select
        
        MyColl.Add MyData
        
    Next Counter
    
    For Counter = 1 To DataArrayRows - 1
    
        FruitArray(Counter, 1) = MyColl.Item(Counter).Fruit
        
    Next Counter
    
    Sheet1.Cells(2, 2).Resize(DataArrayRows, 1).Value = FruitArray()
    
End Sub

The code above works but seems to take a long time on this line:

For Counter = 1 To DataArrayRows - 1
    
        FruitArray(Counter, 1) = MyColl.Item(Counter).Fruit
        
    Next Counter

Why is that?

Thanks

PS I know I could use a VLookup instead!

 

[StephenCrump]

Try replacing your loop with:

Dim c As clsData
Dim i As Long

'....

For Each c In MyColl
    i = i + 1
    FruitArray(i, 1) = c.Fruit
Next c

That should significantly improve the speed. You might get further marginal improvement by declaring FruitArray as String rather than Variant.

Just by the way, there are a couple of references to pDayofWeek in your Class Module that should be pFruit.
At the moment, your code just happens to work because of the particular order you happen to be calling Let/Get.

 

[tiredofit]

Try replacing your loop with:

Dim c As clsData
Dim i As Long

'....

For Each c In MyColl
    i = i + 1
    FruitArray(i, 1) = c.Fruit
Next c

That should significantly improve the speed. You might get further marginal improvement by declaring FruitArray as String rather than Variant.

Just by the way, there are a couple of references to pDayofWeek in your Class Module that should be pFruit.
At the moment, your code just happens to work because of the particular order you happen to be calling Let/Get.

Thanks for your tip, it really was much faster.

Can you please explain why mine was slow?

 

[tiredofit]

Try replacing your loop with:

Dim c As clsData
Dim i As Long

'....

For Each c In MyColl
    i = i + 1
    FruitArray(i, 1) = c.Fruit
Next c

That should significantly improve the speed. You might get further marginal improvement by declaring FruitArray as String rather than Variant.

Just by the way, there are a couple of references to pDayofWeek in your Class Module that should be pFruit.
At the moment, your code just happens to work because of the particular order you happen to be calling Let/Get.

Also why did you define another instance of ClsData by the variable c?

Could you not have used the variable MyData alreay defined, ie:

For Each MyData In Mycoll

    I = I + 1
    
    FruitArray(I, 1) = MyData.Fruit 

Next C

Thanks

 

[StephenCrump]

Which is better: A For loop or a For Each Loop? The answer is a definitive: it depends.

But for Collections, For Each is much faster. I haven't checked the relative timing myself, but have a look here for example: https://analystcave.com/vba-for-loop-vs-for-each-loop/

I think of it this way:

With a For Each you're simply asking VBA: Loop through that Collection you have stored.

With a For, you're saying:
Find me the first item.
Now find me the 2nd item
....
Now find me the 299,999th item
Now find me the 300,000th item
....

In terms of the detail, you're quite right about MyData. I could also have used your existing Counter rather than creating a new i.

 

[tiredofit]

Which is better: A

​

For

loop or a ​

For Each

Loop? The answer is a definitive: it depends.

But for Collections, ​

For Each

is much faster. I haven't checked the relative timing myself, but have a look here for example: ​

https://analystcave.com/vba-for-loop-vs-for-each-loop/

I think of it this way:

With a ​

For Each

you're simply asking VBA: Loop through that Collection you have stored.

With a For, you're saying:
Find me the first item.
Now find me the 2nd item
....
Now find me the 299,999th item
Now find me the 300,000th item
....

In terms of the detail, you're quite right about ​

MyData

. I could also have used your existing ​

Counter

rather than creating a new ​

i

.​

Thanks

Just a quick note:

I added this line in by accident:

Set MyData = Nothing​

but it still worked!

I thought the moment MyData is cleared out, the loop wouldn't work.

Option Explicit


Sub Test()
    
    Dim DataArray() As Variant
    
    DataArray = Sheet1.Cells(1, 1).CurrentRegion.Value
    
    Dim DataArrayRows As Long
    
    DataArrayRows = UBound(DataArray(), 1)
    
    Dim FruitArray() As Variant
    
    ReDim FruitArray(1 To DataArrayRows, 1 To 1) As Variant
    
    Dim Counter As Long
    
    Dim MyData As ClsData
    
    Dim MyColl As Collection
    Set MyColl = New Collection
    
    For Counter = 2 To DataArrayRows
            
        Set MyData = New ClsData
        
        MyData.DayOfWeek = DataArray(Counter, 1)
        
        Select Case MyData.DayOfWeek
        
            Case "Monday"
                
                MyData.Fruit = "Orange"
                
            Case "Tuesday"
                
                MyData.Fruit = "Apple"
            
            Case Else
            
                MyData.Fruit = "Banana"
                
        End Select
        
        MyColl.Add MyData
        
    Next Counter
    
    Set MyData = Nothing 'ADDED BY MISTAKE
    
    Dim i As Long
    
    For Each MyData In MyColl

        i = i + 1

        FruitArray(i, 1) = MyData.Fruit

    Next MyData

    Sheet1.Cells(2, 2).Resize(DataArrayRows, 1).Value = FruitArray()


End Sub

 

[StephenCrump]

I added this line in by accident:

Set MyData = Nothing​

but it still worked!

I thought the moment MyData is cleared out, the loop wouldn't work.

So why is that?

When you declare: Dim MyData As ClsData, you're asking VBA to set aside enough memory to store one instance of ClsData. You'll provide the content later in the code.

Inside your first loop (below), you're using that MyData variable to populate your Collection. At the end of the loop, the Collection will be storing all the multiple instances of ClsData. But MyData still contains only a single instance, i.e. with the .DayOfWeek and .Fruit properties that were assigned in the last iteration of the loop.

For counter = 2 To DataArrayRows    
    Set MyData = New ClsData
    MyData.DayOfWeek = DataArray(counter, 1)
    MyData.Fruit = "Orange" 'for example
    MyColl.Add MyData
Next counter

You may be misinterpeting this line: Set MyData = New ClsData

MyData still contains only a single instance of ClsData, but by setting it to a New ClsData you have cleared its contents. In fact, given that the next two lines above overwrite both the .DayOfWeek and .Fruit properties, the Set MyData = New ClsData line is redundant in this simple example.

Your second loop starts:

For Each MyData In MyColl

Now you are re-cycling the MyData variable for a different purpose. On the first iteration of this loop, MyData will refers to the first ClsData element stored in the Collection, i.e. overwriting whatever the previous content of MyData might have been.

These two steps won't be impacted by what you do with the MyData variable in the interim, e.g. in your case, you accidentally set it to Nothing, or you could have set it to another spurious instance with random .DayOfWeek and .Fruit properties.

 

[tiredofit]

So why is that?

When you declare: Dim MyData As ClsData, you're asking VBA to set aside enough memory to store one instance of ClsData. You'll provide the content later in the code.

Inside your first loop (below), you're using that MyData variable to populate your Collection. At the end of the loop, the Collection will be storing all the multiple instances of ClsData. But MyData still contains only a single instance, i.e. with the .DayOfWeek and .Fruit properties that were assigned in the last iteration of the loop.

For counter = 2 To DataArrayRows    
    Set MyData = New ClsData
    MyData.DayOfWeek = DataArray(counter, 1)
    MyData.Fruit = "Orange" 'for example
    MyColl.Add MyData
Next counter

You may be misinterpeting this line: Set MyData = New ClsData

MyData still contains only a single instance of ClsData, but by setting it to a New ClsData you have cleared its contents. In fact, given that the next two lines above overwrite both the .DayOfWeek and .Fruit properties, the Set MyData = New ClsData line is redundant in this simple example.

Your second loop starts:

For Each MyData In MyColl

Now you are re-cycling the MyData variable for a different purpose. On the first iteration of this loop, MyData will refers to the first ClsData element stored in the Collection, i.e. overwriting whatever the previous content of MyData might have been.

These two steps won't be impacted by what you do with the MyData variable in the interim, e.g. in your case, you accidentally set it to Nothing, or you could have set it to another spurious instance with random .DayOfWeek and .Fruit properties.

Thanks a lot for you very detailed explanation.

 

[tiredofit]

So why is that?

When you declare: Dim MyData As ClsData, you're asking VBA to set aside enough memory to store one instance of ClsData. You'll provide the content later in the code.

Inside your first loop (below), you're using that MyData variable to populate your Collection. At the end of the loop, the Collection will be storing all the multiple instances of ClsData. But MyData still contains only a single instance, i.e. with the .DayOfWeek and .Fruit properties that were assigned in the last iteration of the loop.

For counter = 2 To DataArrayRows    
    Set MyData = New ClsData
    MyData.DayOfWeek = DataArray(counter, 1)
    MyData.Fruit = "Orange" 'for example
    MyColl.Add MyData
Next counter

You may be misinterpeting this line: Set MyData = New ClsData

MyData still contains only a single instance of ClsData, but by setting it to a New ClsData you have cleared its contents. In fact, given that the next two lines above overwrite both the .DayOfWeek and .Fruit properties, the Set MyData = New ClsData line is redundant in this simple example.

Your second loop starts:

For Each MyData In MyColl

Now you are re-cycling the MyData variable for a different purpose. On the first iteration of this loop, MyData will refers to the first ClsData element stored in the Collection, i.e. overwriting whatever the previous content of MyData might have been.

These two steps won't be impacted by what you do with the MyData variable in the interim, e.g. in your case, you accidentally set it to Nothing, or you could have set it to another spurious instance with random .DayOfWeek and .Fruit properties.

I think I finally understand it.

I think my confusion arises by using MyData in the second loop as well. Am I correct in saying it has NOTHING to do with the "first" MyData?

I experimented as follows:

Dim abc

For Each abc In MyColl

        i = i + 1

        FruitArray(i, 1) = abc.Fruit

    Next abc

​

and it still worked, meaning the variable (abc in this case) has nothing to do with classes. It's just a variable to "extract" the elements in the collection, MyColl?

 

[StephenCrump]

I think my confusion arises by using MyData in the second loop as well. Am I correct in saying it has NOTHING to do with the "first" MyData?

In a sense, yes.

We're reusing a variable called MyData. It's the same variable, i.e. the same bit of space somewhere in memory, but when we use it the second time, the content we put in to that variable has nothing to do with the content we put into it previously.

The same principle is used in this bit of code, which re-uses the variable i:

For i = 1 To 100
    'Do something
Next i

'..

For i = 3 To 6
    'Now do something completely different
Next i

At the end of the first loop, i has the value 101. In the first iteration of the second loop, i will be assigned the value 3. It's the same variable i, but its contents have nothing to do with what was there previously.

If we had used j instead of i as the second loop counter, i would keep its value 101 to the end of the Sub, or until it was otherwise used again. But we don't need to know this anymore, hence our ability to re-use.

Is it OK to reuse variables? You'll get some differences of opinion from coders on that question. I'm happy to do so in simple examples like these, because there is no scope for ambiguity. In bigger coding exercises, I err on the side of caution. Even though it might be obvious to me (and at that particular time, rather than five years later!) that I can re-use a variable, Count, say, for different purposes. I'll probably create variables called CountDescription1, CountDescription2 etc, to self-document the code.