Removing Items in a List While Iterating Through It with For Each Loop

Question

I have a list named NeededList I need to check each item in this list to see if it exists in my database. If it does exist in the database I need to remove it from the list. But I can't change the list while I'm iterating through it. How can I make this work?

Here is my code so far:

For Each Needed In NeededList
        Dim Ticker = Needed.Split("-")(0).Trim()
        Dim Year = Needed.Split("-")(1).Trim()
        Dim Period = Needed.Split("-")(2).Trim()
        Dim Table = Needed.Split("-")(3).Trim()
        Dim dr As OleDbDataReader
        Dim cmd2 As New OleDb.OleDbCommand("SELECT * FROM " & Table & " WHERE Ticker = ? AND [Year] = ? AND Period = ?", con)
        cmd2.Parameters.AddWithValue("?", Ticker)
        cmd2.Parameters.AddWithValue("?", Year)
        cmd2.Parameters.AddWithValue("?", Period)
        dr = cmd2.ExecuteReader
        If dr.HasRows Then
            NeededList.Remove(Needed)
        End If
Next

Solution 1

No you can't do that using a for each, but you can do that using the old fashioned for .. loop.
The trick is to start from the end and looping backwards.

For x = NeededList.Count - 1 to 0 Step -1
    ' Get the element to evaluate....
    Dim Needed = NeededList(x)
    .....
    If dr.HasRows Then
        NeededList.RemoveAt(x)
    End If
Next

You need to approach the loop in this way because you don't risk to skip elements because the current one has been deleted.

For example, suppose that you remove the fourth element in the collection, after that, the fifth element becomes the fourth. But then the indexer goes up to 5. In this way, the previous fifth element (now in fourth position) is never evaluated. Of course you could try to change the value of the indexer but this ends always in bad code and bugs waiting to happen.

Solution 2

Go for safe and make a copy with ToList():

For Each Needed In NeededList.ToList()
    Dim Ticker = Needed.Split("-")(0).Trim()
    ...
    If dr.HasRows Then
        NeededList.Remove(Needed)
    End If
Next

Solution 3

You can use a For loop iterating through every index with Step -1.

For i as Integer = NeededList.Count - 1 to 0 Step -1

    Dim Needed = NeededList(i)

    'this is a copy of your code
    Dim Ticker = Needed.Split("-")(0).Trim()
    Dim Year = Needed.Split("-")(1).Trim()
    Dim Period = Needed.Split("-")(2).Trim()
    Dim Table = Needed.Split("-")(3).Trim()

    Dim dr As OleDbDataReader
    Dim cmd2 As New OleDb.OleDbCommand("SELECT * FROM " & Table & " WHERE Ticker = ? AND [Year] = ? AND Period = ?", con)
    cmd2.Parameters.AddWithValue("?", Ticker)
    cmd2.Parameters.AddWithValue("?", Year)
    cmd2.Parameters.AddWithValue("?", Period)
    dr = cmd2.ExecuteReader

    'MODIFIED CODE
    If dr.HasRows Then NeededList.RemoveAt(i)

Next i

Solution 4

The contents of an array (or anything else you can fast enumerate with For Each can not be modified with a For Each loop. You need to use a simple For loop and iterate through every index.

Hint: Because you'll be deleting indexes, I suggest starting at the last index and work your way toward the first index so you don't skip over one every time you delete one.

View more solutions

Comments

[gromit1]

How to I split each item apart with this method?

[gromit1]

How do I split each item apart with this method?

[gromit1]

How do I split each item apart with this method?

[Steve]

You can split the Needed variable as you have done in your original code.

[gromit1]

When I try that it says that Needed hasn't been declared because I was declaring it in this line of code For Each Needed In NeededList which I've replaced.

[nhgrif]

Dim Needed = NeededList(x) as Steve has in the second line of the code there.

[gromit1]

Got it. Sorry for being dense!

[tezzo]

Answer updated. You can delete from NeedList without problem because using a different For loop you start from the last element in it.

[gromit1]

I get this error "x" is not declared. It may be inaccessible due to its protection level.

[robnick]

This is a nice simple solution and does not require the backwards iterating which is not handy if you are wanting to process in FIFO order. I know there are lot of different ways to "skin the cat" but this one worked nicely for me.

[Pachanka]

Never delete in a for boucle, use while instead like code Dim i as integer =0 while i<>list.count-1 //some code if isNeedToDelete then list(i).remove else i+=1 end if end while

[PeterCo]

I have lost hours with my removing For Each loop until I finally found your .ToList. Thanks for sharing it!