1

I am currently working on a C# WPF project. I have a list which uses a class to store multiple values. The class is called DBTables and has the following inside:

class DBTables
{
 public string selDatabase { get; set; }
 public string selTable { get; set; }
}

I am creating a new instance of the list with the following code

List<DBTables> tableArr = new List<DBTables>();

I am adding new items to the List without any problems but the problem I am having is when it comes to removing an item from the list.

A an item is added to the list when a checkbox is selected the item is added and when the checkbox is unchecked the item needs to be removed. Each time the checkbox is checked two values are added using the following code:

private void addBackupArray(string table)
{
    backupArr.Add(new DBTables
    {
        selDatabase = selectedDatabase,
        selTable = table
    });
}

When the check box is unchecked the values at the position need to be removed and I have sort of got it working but after it has removed the item it then displays the error 'InvalidOperationException, collection was modified; enumeration may not execute'.

Below is the code that I am currently using to remove the item from the list.

private void removeBackupArray(string table)
{
    int i = 0;
    foreach (DBTables tables in backupArr)
    {
        if (selectedDatabase == tables.selDatabase && table == tables.selTable)
        {
            backupArr.RemoveAt(i);
            i = 0;
        }
        i++;
    }
}

The code above iterates through the values in the list and based on an if statement of whether the two variables match the value found in the list it removes it at the current position of the counter i.

How can I get round this issue so I can remove the item without getting the error.

Thanks for any help you can provide.

Boardy
  • 35,417
  • 104
  • 256
  • 447

3 Answers3

3

Change the foreach to normal for loop will fix the issue:

for (int tablesIndex = 0; tablesIndex < backupArr.Count; tablesIndex++)
{
    var tables = backupArr[tablesIndex];

    if (selectedDatabase == tables.selDatabase && table == tables.selTable)
    {
        backupArr.RemoveAt(tablesIndex);
        tablesIndex--;
    }
}
Jalal Said
  • 15,906
  • 7
  • 45
  • 68
  • When I try that it comes up with a compile error saying that it is expecting a semicolon (;) – Boardy Aug 07 '11 at 15:14
  • 2
    Just want to add a little explanation, foreach is a readonly loop and should be used for reading the values and not modifying it. – Deepansh Gupta Aug 07 '11 at 15:15
  • @Boardy: I added a code snippet check it. – Jalal Said Aug 07 '11 at 15:16
  • Thanks worked great bit of a newbie when it comes to for and foreach loops – Boardy Aug 07 '11 at 15:21
  • Why are you reseting `i` to `0` after you remove an item? This will fail miserably is there are more items that match the condition. – svick Aug 07 '11 at 15:35
  • If I don't reset i and also tablesindex to actually -1 so that when it does ++ it resets it to 0 otherwise it will leave the first record in the list which should have been deleted. – Boardy Aug 07 '11 at 15:42
  • @svick: I didn't reset any thing! it is his code, I just copy paste the code and show hem how to convert to from `foreach` to `for` loop... – Jalal Said Aug 07 '11 at 15:43
  • @svick: Also note that I declared a new `int tablesIndex = 0` and use it in the loop, the `i` here is from his code, You should consider asking him about it, not `-1` at the light speed! – Jalal Said Aug 07 '11 at 15:50
  • @Jalal, I don't care who originally wrote it. I think the code you posted is wrong. – svick Aug 07 '11 at 16:08
  • @svick, Jalal code is correct. If i isn't reset to 0 then the first item in the List which should be deleted, doesn't get deleted which is why I put i = 0; – Boardy Aug 07 '11 at 16:20
  • @svick: Ok I fixed it up, you have a point here that maybe this code is reviewed from another... – Jalal Said Aug 07 '11 at 16:25
  • @Boardy: I updated the answer check it. – Jalal Said Aug 07 '11 at 16:25
  • @Jalal, @Boardy: the for loop should traverse the list in reverse when items are being removed from the list, otherwise the loop will end prematurely as the `Count` is modified, potentially skipping other valid items. Traversing in reverse from `Count - 1` to zero avoids this problem. For a clear example check out [my answer to another question](http://stackoverflow.com/questions/1582285/how-to-remove-elements-from-a-generic-list-while-iterating-over-it/1582317#1582317). – Ahmad Mageed Aug 07 '11 at 16:32
  • @Ahmad: You are right I forget about adding `tablesIndex--;` after the last edit to the answer ;) – Jalal Said Aug 07 '11 at 16:35
0

Changed your foreach to a for loop. The foreach uses an enumerator to iterate over all of the objects in the List. You can't change the contents of the enumerator within the foreach or you'll get the error you see.

Give this a try instead

int i;
for (i = 0; i < backupArr.Count; i++)
{
    DBTables tables = backupArr[i];
    if (selectedDatabase == tables.selDatabase && table == tables.selTable)
    {
        break;
    }
}

backupArr.RemoveAt(i);
Brian Dishaw
  • 5,767
  • 34
  • 49
0

A neater solution could be to use a linq like so:

        DBTables tables = backupArr.Where(t => t.selDatabase == selectedDatabase && t.selTable == table).SingleOrDefault();
        if (tables != null)
            backupArr.Remove(tables);
jdavies
  • 12,784
  • 3
  • 33
  • 33