20

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
gromit1
  • 577
  • 2
  • 14
  • 36

7 Answers7

41

No you can't do that using For Each. The reason is explained in this answer where the author tries to pinpoint the logical problems and the practical inefficiencies that such implementation could trigger.

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 want to skip an element because the current one has just been deleted.

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

Steve
  • 213,761
  • 22
  • 232
  • 286
  • How do I split each item apart with this method? – gromit1 Oct 08 '13 at 15:47
  • 1
    You can split the `Needed` variable as you have done in your original code. – Steve Oct 08 '13 at 15:50
  • 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. – gromit1 Oct 08 '13 at 15:51
  • 2
    `Dim Needed = NeededList(x)` as Steve has in the second line of the code there. – nhgrif Oct 08 '13 at 15:51
  • Got it. Sorry for being dense! – gromit1 Oct 08 '13 at 15:52
  • 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 – Pachanka Feb 22 '16 at 16:42
21

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
H H
  • 263,252
  • 30
  • 330
  • 514
  • 1
    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. – robnick Mar 20 '15 at 03:09
  • I have lost hours with my removing `For Each` loop until I finally found your `.ToList`. Thanks for sharing it! – PeterCo Nov 09 '16 at 17:20
4

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
tezzo
  • 10,858
  • 1
  • 25
  • 48
3

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.

nhgrif
  • 61,578
  • 25
  • 134
  • 173
0

No you can not remove from a List that you are working on e.g.

For Each Str As String In listOfStrings 
                    If Str.Equals("Pat") Then
                        Dim index = listOfStrings.IndexOf(Str)
                        listOfStrings .RemoveAt(index)
                    End If
                Next

But this way will work make a copy of your list and delete from it e.g.

For Each Str As String In listOfStrings 
                    If Str.Equals("Pat") Then
                        Dim index = listOfStringsCopy.IndexOf(Str)
                        listOfStringsCopy.RemoveAt(index)
                    End If
                Next
General Grievance
  • 4,555
  • 31
  • 31
  • 45
Pec1983
  • 346
  • 4
  • 8
0

You can also invert the order of the list's elements and still use For Each using the IEnumerable Cast and Reverse extensions.

Simple example using a List(Of String):

For Each Needed In NeededList.Cast(Of List(Of String)).Reverse()
    If dr.HasRows Then
        NeededList.Remove(Needed)
    End If
Next
mikro
  • 505
  • 1
  • 6
  • 12
0

How about this (no iteration needed):

NeededList = (NeededList.Where(Function(Needed) IsNeeded(Needed)).ToList

Function IsNeeded(Needed As ...) As Boolean
    ...
    Return Not dr.HasRows
End Function
C66
  • 1