CODEWITHSUNDEEP
excelvbacollections
arnavlohe15
arnavlohe15

Reputation: 334

Runtime Error 5: Removing Certain Items from VBA Collection

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?

Upvotes: 0

Views: 462

Answers (1)

Mathieu Guindon
Mathieu Guindon

Reputation: 71187

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!

Upvotes: 2

Related Questions