Runtime Error 5: Removing Certain Items from VBA Collection

Question

I have created an Excel collection from an Excel Column by using a FOR iteration through that column. However, this collection includes the header and blank columns. I wish to remove these blank columns and the header from the collection and do not know how. Based on my code, I keep receiving a Runtime Error 5

I have tried to use a For Each iteration through the collection and collection_name.Remove element based on a conditional which you can see below, but this has not worked.

Public Sub outer_lÖÖp()


'creating a collection of the policy numbers
Sheets("MappingSheet").Select
Dim policynums As New Collection
Dim i As Integer
For i = 1 To 100
    policynums.Add (Cells(i, 1))
Next i

'getting rid of "Policy Number" and empty strings from collection
Worksheets("GL").Activate
For Each element In policynums
    If element = "Policy Number" Or element = "" Then
        policynums.Remove element
    End If
Next element

For Each element In policynums
    MsgBox (CStr(element))
Next element


End Sub

This keeps returning a Runtime Error 5, invalid procedure element or call. How can I solve my problem and remove all irrelevant strings from this collection?

Why don't you skip adding them on the first loop? Instead looping again to delete, insert your if statement into the adding loop to avoid adding the items you don't need. — Damian, Jun 03 '19 at 15:50
You get that error because if you are are not identifying the element to remove by its index you must use keys when populating the collection. Follow Damian's advice. — SJR, Jun 03 '19 at 15:59
@Damian when I tried that, I was unable to verify that the elements inserted correctly. The message box was not working — arnavlohe15, Jun 03 '19 at 16:01
Don't `MsgBox` in a loop. Delete that loop, and hit F8 to place a breakpoint on `End Sub` instead. When the breakpoint is hit, bring up the *locals* toolwindow and inspect the contents of the `policynums` object. Place the breakpoint earlier to break earlier, e.g. in the `i` loop. FWIW drop these extraneous parentheses, they're dangerous. And qualify that `Cell` call with the `Worksheet` you're currently `Select`ing - there's no need to `Select` anything here. — Mathieu Guindon, Jun 03 '19 at 16:04
As for the empty values, assuming that `100` just stands for "whatever number greater than the number of actual rows I need", what you want is to [get the actual number of rows](https://stackoverflow.com/a/11169920/1188513) instead. — Mathieu Guindon, Jun 03 '19 at 16:24

score 2 · Answer 1 · answered Jun 03 '19 at 16:17

The data is coming from a specific worksheet. First step is to get that object.

Dim sheet As Worksheet
'TODO make this work off a Workbook object so we don't need to care what workbook is currently active.
Set sheet = ActiveWorkbook.Worksheets("MappingSheet")

Next, we want a collection object - avoid As New, it makes an auto-instantiated object, and that has consequences you really don't want to be dealing with.

Dim elements As Collection
Set elements = New Collection

Next we loop 1 To 100. While that does fit the Integer range, we avoid the legacy 16-bit integer type, especially for something like a row number, which can be in the millions: use a Long instead, a 32-bit integer type.

Dim currentRow As Long
For currentRow = 1 To 100 'TODO determine how many rows we need before we start looping.
    elements.Add sheet.Cells(currentRow, 1).Value
Next

Now, why iterate the collection again, just to remove things that shouldn't have been added in the first place?

Dim currentRow As Long
For currentRow = 1 To 100 'TODO determine how many rows we need before we start looping.
    Dim cell As Range
    Set cell = sheet.Cells(currentRow, 1)
    If Not IsError(cell.Value) Then
        If Trim$(cell.Value) <> vbNullString And cell.Value <> "Policy Number" Then
            elements.Add sheet.Cells(currentRow, 1).Value
        End If
    End If
Next

If the only reason to check for the "Policy Number" literal is to avoid the first record which is your row heading, remove that check, and start looping at row 2 instead -- assuming the table header is in row 1:

For currentRow = 2 To 100 'TODO determine how many rows we need before we start looping.
    Dim cell As Range
    Set cell = sheet.Cells(currentRow, 1)
    If Not IsError(cell.Value) Then
        If Trim$(cell.Value) <> vbNullString Then
            elements.Add sheet.Cells(currentRow, 1).Value, Key:="Row" & currentRow
        End If
    End If
Next

Notice the Key named argument (only named for clarify in this post - it doesn't have to be named.. although it doesn't hurt either): if you wanted to remove the item keyed with "Row12" from the collection, you would do this:

elements.Remove "Row12"

Lastly, MsgBox (and any other procedure call for which you aren't capturing the return value) should look like this:

MsgBox element

The extraneous parentheses have a syntactic meaning that you are probably not expecting. This wouldn't compile:

MsgBox (element, vbInformation)

Because the parentheses are telling VBA to evaluate their contents as a value expression, and pass the result of that expression ByVal to the invoked procedure. But (foo, bar) isn't a valid expression, so the code refuses to compile.

In any case, rule of thumb you want to Debug.Print the contents of a collection, or just use the debugger toolwindows to inspect its contents - a MsgBox in a loop is obnoxiously annoying!

Helpful coaching, possibly not only for beginners :-) Side Note: `Option Explicit` would prevent to *'forget'* declarations (`element`) in OP. — T.M., Jun 03 '19 at 16:38

Runtime Error 5: Removing Certain Items from VBA Collection

1 Answers1