Safely Removing DataRow In ForEach

Question

I don't understand why this code does not work.

foreach (DataRow dataRow in dataTable.Rows)
{
    if (true)
    {
        dataRow.Delete();
    }
}

Solution 1

Even though DataRow.Delete doesn't modify the state of the collection, Microsoft documentation states that you shouldn't call it while iterating over the collection:

Neither Delete nor Remove should be called in a foreach loop while iterating through a DataRowCollection object. Delete nor Remove modify the state of the collection.

The best solution is usually to create a separate collection (e.g. a List<DataRow>) of items you want to remove, and then remove them after you've finished iterating.

This is also the solution for situations where you want to remove items from a collection, as most collections in .NET don't allow you to change the contents of the collection while you're iterating over it.

Solution 2

Safest way - use for loop

for (int i = datatable.Rows.Count - 1; i >= 0; i--) 
{
    if (true)
    {
        datatable.Rows[i].Delete();
    }
}

Don't forget to AcceptChanges to remove all marked rows:

datatable.AcceptChanges();

Solution 3

You cannot modify a collection while you're iterating on it using a foreach statement.

you can try something like that :

List<DataRow> deletedRows = new List<DataRow>();

foreach (DataRow dataRow in dataTable.Rows)
{
    if(true) deletedRows.Add(dataRow);
}

foreach(DataRow dataRow in deletedRows)
{
    dataRow.Delete();
}

Solution 4

If you call the delete method you just have to call AcceptChanges() on the table you are modifying, after the foreach loop.

foreach (DataRow dataRow in dataTable.Rows)
{
    if (true)
    {
        dataRow.Delete();
    }
}

dataTable.AcceptChanges();

The delete method simply marks the row for deletion.

http://msdn.microsoft.com/en-us/library/system.data.datarow.delete%28v=VS.90%29.aspx

Solution 5

may be my answer not longer useful. exception throw in Foreach with DataRow only appear in .Net 2.0 and earlier, the reason is description at msdn http://msdn.microsoft.com/en-us/library/system.data.datarow.delete(v=vs.80).aspx

If the RowState of the row is Added, the row is removed from the table.

The RowState becomes Deleted after you use the Delete method. It remains Deleted until you call AcceptChanges.

A deleted row can be undeleted by invoking RejectChanges.

to pass this problem you can call DataTable.AcceptChanges() before using foreach

View more solutions

Comments

[AllenG]

Incorrect. You cannot modify a collection while iterating through it with a "foreach" You can use a standard 'for' loop. eg: for(int i = datatable.Rows.length - 1; i >= 0; i --) { //remove rows }

[Thibault Falise]

@AllenG : Thanks, I added the info.

[Developer]

Deletion is possible while iteration is on stackoverflow.com/questions/3150216/…

[Thibault Falise]

@fzshah76 : No, in your example, using the Select() method creates a new collection containing all rows of the previous collection. You are not iterating on the original collection, which contains the row you are deleting.

[Developer]

@ Thibault Falise: Thanks for clarifying :)

[Eternal21]

I agree with your answer, however how do you explain this: msdn.microsoft.com/en-us/library/… "Delete should not be called in a foreach loop while iterating through a DataRowCollection object. Delete modifies the state of the collection." Error in MSDN documentation?

[peter]

Nice. Use 'datatable.AcceptChanges()' to really delete the rows afterwards. Otherwise, they are still in the datatable - with a RowState of 'deleted'

[HerrimanCoder]

Nice solution, not overly hackish, works great. But since jumping through this extra little hoop is so easy it makes me wonder why M$ doesn't just make this natively possible within the iteration, even if under the hood they're jumping through the hoop for us.

[RBT]

From review queue: May I request you to please add some more context around your answer. Code-only answers are difficult to understand. It will help the asker and future readers both if you can add more information in your post.

[Manan Shah]

why the "if(true)" , is that necessary ?

[VMAtm]

if (true) is a placeholder to check, do you need to delete the row. Otherwise all your rows from a datatable will be removed

[Moses]

dataRow.Delete does not remove the row from the collection and does not cause the collections size to change nor does it cause the collection to be reordered so a foreach(DataRow row in table.Rows) { row.Delete(); } as long as acceptChanges is not called on an object that would cause the row collection to be changed in length.

[Jon Skeet]

@Moses: Hmm... I don't have time to investigate this now, unfortunately... The documentation does state: "Neither Delete nor Remove should be called in a foreach loop while iterating through a DataRowCollection object. Delete nor Remove modify the state of the collection." I'll edit that in.

[Cee McSharpface]

not a native speaker, still that second nor does not seem to make sense. what is that supposed to mean?

[Jon Skeet]

@CeeMcSharpface: It means "Delete doesn't modify the state of the collection, and Remove doesn't modify the state of the collection."

[Cee McSharpface]

thanks. got it. so the recommendation is to not do it because invoking methods on foreach loop variables that look like they could modify the enumerable is a bad pattern in general, and not because it would be harmful in this particular scenario.

[Jon Skeet]

@CeeMcSharpface: To be honest, I can't remember the details 11 years later...

[Cee McSharpface]

understandable. still relevant though, I had exactly this in a (professional, business) code review today, criticized it and wanted to include an informed "why" - which I could, thanks to this answer and clarifications.

[Daniel Wu]

@Moses I used to do this on NET 4.5. Upgrading to NET 4.8 causes me headache, which is apparently because I do this (iterating through foreach, performing DataRow.Delete(), calling DataTable.AcceptChanges() at the end). I think this method used to work in the past, but now we really should change it.