AJD
AJD

Reputation: 2438

Type mismatch trying to set data in an object in a collection

I am getting Runtime Error 13 when trying to update an object stored in a collection. Here is a minimal example.

The class (Class2) of the objects to be stored in the collection.

Option Explicit

Private pHasA As Boolean
Private pHasB As Boolean
Private pSomeRandomID As String


Property Get HasA() As Boolean
    HasA = pHasA
End Property


Property Get HasB() As Boolean
    HasB = pHasB
End Property


Property Let HasA(propValue As Boolean)
    pHasA = propValue
End Property


Property Let HasB(propValue As Boolean)
    pHasB = propValue
End Property

Property Let RandomID(propValue As String)
    pSomeRandomID = propValue
End Property


Sub SetHasValues(key As String)
    Select Case key
        Case "A"
            pHasA = True
        Case "B"
            pHasB = True
    End Select
End Sub

Minimal code that reproduces the error:

Option Explicit

Private Sub TestCollectionError()

Dim classArray As Variant
Dim classCollection As Collection
Dim singleClass2Item As Class2
Dim iterator As Long

    classArray = Array("A", "B", "C")
    Set classCollection = New Collection

    For iterator = LBound(classArray) To UBound(classArray)
        Set singleClass2Item = New Class2
        singleClass2Item.RandomID = classArray(iterator)
        classCollection.Add singleClass2Item, classArray(iterator)
    Next iterator

    Debug.Print "Count: " & classCollection.Count

    singleClass2Item.SetHasValues "A" ' <-- This code works fine.
    Debug.Print "New Truth values: " & singleClass2Item.HasA, singleClass2Item.HasB

    For iterator = LBound(classArray) To UBound(classArray)
        classCollection(classArray(iterator)).RandomID = classArray(iterator)
        classCollection(classArray(iterator)).SetHasValues classArray(iterator)  '<-- Type mismatch on this line.
    Next iterator

 '***** outputs
'''Count: 3
'''New Truth values: True      False
' Error dialog as noted in the comment above

End Sub

While the code above appears a little contrived, it is based on some real code that I am using to automate Excel.

I have searched for answers here (including the following posts), but they do not address the simple and non-ambiguous example that I have here. The answers that I have found have addressed true type mismatches, wrong use of indexing or similar clear answers.

Upvotes: 1

Views: 1021

Answers (2)

freeflow
freeflow

Reputation: 4355

@ADJ That's annoying, but perhaps the example below will allow you to start making a case for allowing RubberDuck.

I've upgraded your code using ideas and concepts I've gained from the rubberduck blogs. The code now compiles cleanly and is (imho) is less cluttered due to fewer lookups.

Key points to note are

  1. Not relying on implicit type conversions
  2. Assigning objects retrieved from collections to a variable of the type you are retrieving to get access to intellisense for the object
  3. VBA objects with true constructors (the Create and Self functions in class2)
  4. Encapsulation of the backing variables for class properties to give consistent (and simple) naming coupled with intellisense.

The code below does contain Rubberduck Annotations (comments starting '@)

Updated Class 2

Option Explicit
'@Folder("StackOverflowExamples")
'@PredeclaredId

Private Type Properties

    HasA                    As Boolean
    HasB                    As Boolean
    SomeRandomID            As String

End Type

Private p                   As Properties


Property Get HasA() As Boolean
    HasA = p.HasA
End Property


Property Get HasB() As Boolean
    HasB = p.HasB
End Property


Property Let HasA(propValue As Boolean)
    p.HasA = propValue
End Property


Property Let HasB(propValue As Boolean)
    p.HasB = propValue
End Property


Property Let RandomID(propValue As String)
    p.SomeRandomID = propValue
End Property


Sub SetHasValues(key As String)
    Select Case key
        Case "A"
            p.HasA = True
        Case "B"
            p.HasB = True
    End Select
End Sub


Public Function Create(ByVal arg As String) As Class2

    With New Class2

        Set Create = .Self(arg)

    End With

End Function


Public Function Self(ByVal arg As String) As Class2

        p.SomeRandomID = arg
        Set Self = Me

End Function

Updated test code

Private Sub TestCollectionError()

Dim classArray                  As Variant
Dim classCollection             As Collection
Dim singleClass2Item            As Class2
Dim my_item                     As Variant
Dim my_retrieved_item           As Class2

    classArray = Array("A", "B", "C")
    Set classCollection = New Collection

    For Each my_item In classArray

        classCollection.Add Item:=Class2.Create(my_item), key:=my_item

    Next

    Debug.Print "Count: " & classCollection.Count
    Set singleClass2Item = classCollection.Item(classCollection.Count)
    Debug.Print "Initial Truth values: " & singleClass2Item.HasA, singleClass2Item.HasB
    singleClass2Item.SetHasValues "A" ' <-- This code works fine.
    Debug.Print "New Truth values: " & singleClass2Item.HasA, singleClass2Item.HasB

    For Each my_item In classArray

        Set my_retrieved_item = classCollection.Item(my_item)
        my_retrieved_item.RandomID = CStr(my_item)
        my_retrieved_item.SetHasValues CStr(my_item)

    Next

End Sub

The 'Private Type Properties' idea comes from a Rubberduck article encapsulating class variable in a 'This' type. My take on this idea is to use two type variable p and s (Properties and State) where p holds the backing variables to properties and s hold variables which represent the internal state of the class. Its not been necessary to use the 'Private Type State' definition in the code above.

VBA classes with constructors relies on the PredeclaredID attribute being set to True. You can do this manually by removing and saving the code, using a text editor to set the attributer to 'True' and then reimporting. The RUbberDuck attribute '@PredeclaredId' allows this to be done automatically by the RubberDuck addin. IN my own code the initialiser for class2 would detect report an error as New should not be used when Classes are their own factories.

BY assigning and intermediate variable when retrieving an object from a class (or even a variant) you give Option Explicit the best change for letting you n=know of any errors.

An finally the Rubberduck Code Inspection shows there are still some issues which need attention

enter image description here

Upvotes: 0

AHeyne
AHeyne

Reputation: 3455

This is caused by the fact, that the parameter of your procedure SetHasValues is implicitely defined ByRef.

Defining it ByVal will fix your problem.

Upvotes: 2

Related Questions