Best way to create a blacklist in vba?

Viewed 67

I am trying to create an arraylist that excludes a few items.

example: list a contains {a,b,c,d,e,f,g} black list is contains {a,c,e} I want my new list to only have {b,d,f,g}

Here's what I've tried so far.

Dim a As Object
Set a = CreateObject("System.Collections.ArrayList")

Dim blacklist As Object
Set blacklist = CreateObject("System.Collections.ArrayList")

Dim newList As Object
Set newList = CreateObject("System.Collections.ArrayList")

a.Add "a"
a.Add "b"
a.Add "c"
a.Add "d"
a.Add "e"
a.Add "f"
a.Add "g"


blacklist.Add "a"
blacklist.Add "c"
blacklist.Add "e"


For Each c In a
    For Each b In blacklist
      If c.value <> b Then newList.Add c.value
    Next b
Next c

It doesn't work and prints the items from a arraylist twice

1 Answers

First of all, the c.value needs to be replaced with c. Now the code runs but is wrong because it will simply add each letter multiple times and you'll end up with duplicates.

You need to replace this:

For Each c In a
    For Each b In blacklist
      If c.value <> b Then newList.Add c.value
    Next b
Next c

with this:

For Each c In a
    found = False
    For Each b In blacklist
        If c = b Then
            found = True
            Exit For
        End If
    Next b
    If Not found Then newList.Add c
Next c

and the code will run as expected.

Of course, the solution provided by @ScottCraner in the comments section is more elegant and you should use it. In the following I will address other aspects of your code as well as other ways of achieving the same result.

You should turn Option Explicit on so that the compiler always forces you to declare your variables. In your code both the b and the c variables are not declared. They should be of type Variant because you use them in a For Each loop but they will have String values actually. Something like:

Option Explicit

Sub Test()
    Dim a As Object
    
    Set a = CreateObject("System.Collections.ArrayList")
    
    Dim blacklist As Object
    Set blacklist = CreateObject("System.Collections.ArrayList")
    
    Dim newList As Object
    Set newList = CreateObject("System.Collections.ArrayList")
    
    a.Add "a"
    a.Add "b"
    a.Add "c"
    a.Add "d"
    a.Add "e"
    a.Add "f"
    a.Add "g"
    
    blacklist.Add "a"
    blacklist.Add "c"
    blacklist.Add "e"
    
    Dim c As Variant
    Dim b As Variant
    Dim found As Boolean
    
    For Each c In a
        found = False
        For Each b In blacklist
            If c = b Then
                found = True
                Exit For
            End If
        Next b
        If Not found Then newList.Add c
    Next c
End Sub

You should name your variables so that they convey more information. a, b, c doesn't mean anything to the reader/maintainer of the code. You've already done it properly with blacklist so just apply the same convention. Also, I would use camelCase to make code easier to read (like you did with newList). Something like:

Option Explicit

Sub Test()
    Dim originalList As Object
    Dim blackList As Object
    Dim newList As Object
    
    Set originalList = CreateObject("System.Collections.ArrayList")
    Set blackList = CreateObject("System.Collections.ArrayList")
    Set newList = CreateObject("System.Collections.ArrayList")
    
    originalList.Add "a"
    originalList.Add "b"
    originalList.Add "c"
    originalList.Add "d"
    originalList.Add "e"
    originalList.Add "f"
    originalList.Add "g"
    
    blackList.Add "a"
    blackList.Add "c"
    blackList.Add "e"
    
    Dim originalItem As Variant
    Dim blackListedItem As Variant
    Dim foundItem As Boolean
    
    For Each originalItem In originalList
        foundItem = False
        For Each blackListedItem In blackList
            If originalItem = blackListedItem Then
                foundItem = True
                Exit For
            End If
        Next blackListedItem
        If Not foundItem Then newList.Add originalItem
    Next originalItem
End Sub

I don't know if you really need to use an ArrayList object. You could achieve the same result with arrays and Collection objects. Using the collection's key hashing would be also more efficient than looping through items (I refer to the second For loop). Something like:

Option Explicit

Sub Test()
    Dim originalList() As Variant
    Dim blackList As Collection
    Dim newList As Collection

    originalList = Array("a", "b", "c", "d", "e", "f", "g")
    Set blackList = New Collection
    With blackList
        .Add Empty, "a" 'Note we only need the key
        .Add Empty, "c"
        .Add Empty, "e"
    End With
    Set newList = New Collection

    Dim originalItem As Variant
    
    On Error Resume Next 'In case key already exists
    For Each originalItem In originalList
        blackList.Item CStr(originalItem) 'Check if key exists
        If Err.Number <> 0 Then
            newList.Add originalItem
            Err.Clear
        End If
    Next originalItem
    On Error GoTo 0
End Sub

Finally, you could use an array library that I've built a while ago. You need to import the LibArrayTools.bas module and then do something like this:

Option Explicit

Sub Test()
    Dim originalList() As Variant
    Dim blackList() As Variant
    Dim newList() As Variant
    
    originalList = Array("a", "b", "c", "d", "e", "f", "g")
    blackList = Array("a", "c", "e")
    newList = Filter1DArray(originalList, CreateFiltersArray("NOT IN", blackList))
End Sub
Related