HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Building a better Collection. Enumerable in VBA

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
collectionbuildingbetterenumerablevba

Problem

VBA's 'Collection' is.... lacking, so, I've been working on a better Collection object that implements many of the features of C#'s Enumerable. This is very much inspired by this question and a follow up to Sorting a Collection and More imitation of Enumerable.

I'm concerned that the API is inconsistent. Some methods like Range and Repeat are meant to be called from a "static" default global instance, while others are to be called on instanced.. instances of 'Enumerable'. I started out writing everything to be static, but the calls felt weird. To clarify, it's the difference between this

Dim c as New Enumerable
Set c = Enumerable.Intersect(collection1,collection2)


and

Set c = collection1.Intersect(collection2)


I opted for the latter wherever it made sense to, (made sense to me) but it makes things inconsistent, because of calls like this.

For each char in Enumerable.Repeat("A",3)
    debug.print char
next

For each number in Enumerable.Range(1,10)
    debug.print "Hello World"
next


This is also a fairly large chunk of code, so I'm interested on how I can better group and organize the code.

Download Available From Google Drive.
Header

VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "Enumerable"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = True
Attribute VB_Exposed = False


Declarations

Option Explicit

Public Enum EnumerableError
    vbMethodNotSupportedError = 438
    EnumerableNotIntializedError = vbObjectError + 6500
    EnumerableInvalidArgumentError
End Enum

Private Const NotInitializedErrorMessage As String = "Collection Property Not Set"
Private Const InvalidArgumentErrorMessage As String = "Invalid Argument."

Private mCollection As Collection
Private mIsSorted As Boolean


Properties

```
Public Property Set Collection(obj As Variant)

If TypeName(obj) = "Collection" Then
Set mCollection = obj
ElseIf TypeName(obj) =

Solution

Design

I think you need to break that class in two. Having instance members on a static class is pretty confusing and bug-prone.

I'd suggest Enumerable to be the static class, with these members (notice source being preferred over collectionObject, and explicit ByRef modifiers and Variant types):

  • Function Contains(ByRef source As Variant, ByVal value As Variant, Optional ByVal compareDefaultProperty As Boolean = False) As Boolean



  • Function First(ByRef source As Variant) As Variant



  • Function Last(ByRef source As Variant) As Variant



  • Function Intersect(ByRef source1 As Variant, ByRef source2 As Variant) As Iteratable



  • Function Distinct(ByRef source As Variant) As Iteratable



  • Function Clone(ByRef source As Variant) As Iteratable



  • Function ToArray(ByRef source As Variant) As Variant



Then, you can implement an Iteratable class with a NewEnum property; the beauty is that the instance variants of the static functions, can simply call on the static versions:

Public Function Contains(ByVal value as Variant, Optional ByVal compareDefaultProperty As Boolean = False) As Boolean
    Contains = Enumerable.Contains(Me, value, compareDefaultProperty)
End Function

Public Function First() As Variant
    First = Enumerable.First(Me)
End Function

Public Function Last() As Variant
    Last = Enumerable.Last(Me)
End Function


And so on and so forth.


Potential Bugs

You're storing a Boolean that "remembers" whether the encapsulated collection is sorted:

Private mCollection As Collection
Private mIsSorted As Boolean


The problem is that...

Public Property Set Collection(obj As Variant)

    If TypeName(obj) = "Collection" Then
        Set mCollection = obj
    ElseIf TypeName(obj) = "Enumerable" Then
        Set mCollection = obj.Collection
    Else
        Set mCollection = New Collection
        Merge obj
    End If

End Property


...the flag is going to lie whenever the Collection property gets set. Simply setting mIsSorted = False in the property setter fixes that.

...but it's more complicated than that:

Public Function Merge(collectionObject As Variant)
' Tries to convert any object passed in to a collection.
' This allows collection *like* objects such as Worksheets and Ranges.

On Error GoTo ErrHandler

    Dim element As Variant
    For Each element In collectionObject
        mCollection.Add element
    Next

    'are we still sorted here?


and:

Public Sub Add(item, Optional Key, Optional Before, Optional After)
    mCollection.Add item, Key, Before, After
    'are we still sorted here?
End Sub


The static functions shouldn't tamper with the instance-level field:

Public Function Range(ByVal startValue As Long, ByVal endValue As Long) As Enumerable 'Collection

    Set mCollection = New Collection
    '...
    Set Range.Collection = mCollection


That should really be a local Collection reference.

Moreover, this code is legal:

Enumerable.Sort


Granted, it's a misuse of the class, but nothing forbids doing this:

Set Enumerable.Collection = New Collection


And abusing the default instance - that's where IsSorted will tell the biggest lies, and break Min and Max implementations.

Splitting the class into a static Enumerable class and a non-static Iteratable class addresses this issue, since the static Enumerable class has no reason to encapsulate an instance-level collection.

Range should raise an error whenever startValue is greater than endValue.


Miscellaneous

  • AssignUnknown isn't used anywhere, I'd remove it.



  • Sort, Min and Max don't make sense on a Collection - it's permitted to compare apples with oranges and bananas. Sort / compare items of an Integer(), a String(), or a List, but not of a Collection.



  • CloneCollection should be called ToCollection; its semantics are very similar to those of ToArray, naming should be just as similar.



  • I'd remove the "Section" comments. 'Collection Wrappers isn't useful. Neither is 'Instance Methods. '"Static" functions to be used with default instance of Enumerable is a little better, but moot if the API gets fixed / split into static + instance API's.



  • Merge should be a Sub. It being a Function that doesn't return anything is quite confusing.



  • I like that Merge works with any array, Collection, or List (did you know that?).



  • I'd add a Clear method, to remove all items at once.

Code Snippets

Public Function Contains(ByVal value as Variant, Optional ByVal compareDefaultProperty As Boolean = False) As Boolean
    Contains = Enumerable.Contains(Me, value, compareDefaultProperty)
End Function

Public Function First() As Variant
    First = Enumerable.First(Me)
End Function

Public Function Last() As Variant
    Last = Enumerable.Last(Me)
End Function
Private mCollection As Collection
Private mIsSorted As Boolean
Public Property Set Collection(obj As Variant)

    If TypeName(obj) = "Collection" Then
        Set mCollection = obj
    ElseIf TypeName(obj) = "Enumerable" Then
        Set mCollection = obj.Collection
    Else
        Set mCollection = New Collection
        Merge obj
    End If

End Property
Public Function Merge(collectionObject As Variant)
' Tries to convert any object passed in to a collection.
' This allows collection *like* objects such as Worksheets and Ranges.

On Error GoTo ErrHandler

    Dim element As Variant
    For Each element In collectionObject
        mCollection.Add element
    Next

    'are we still sorted here?
Public Sub Add(item, Optional Key, Optional Before, Optional After)
    mCollection.Add item, Key, Before, After
    'are we still sorted here?
End Sub

Context

StackExchange Code Review Q#60504, answer score: 11

Revisions (0)

No revisions yet.