4

I am creating a method to handle delete button event inside a DataList, and it's doing the functionality properly, however I get this exception:

Collection was modified; enumeration operation may not execute.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code. 

Exception Details: System.InvalidOperationException: Collection was modified; enumeration operation may not execute.

and this is my code:

protected void delete(object sender, CommandEventArgs e) 
        {
            if ((e.CommandName == "delete") && (e.CommandArgument != null))
            {
                foreach (DataListItem item in DataList2.Items)
                {
                    Label post_IDLabel = (Label)item.FindControl("post_IDLabel");
                    string connStr = ConfigurationManager.ConnectionStrings["MyDbConn"].ToString();
                    SqlConnection conn = new SqlConnection(connStr);
                    SqlCommand cmd = new SqlCommand("delete_post", conn);
                    cmd.CommandType = CommandType.StoredProcedure;
                    int post_ID = Convert.ToInt32(post_IDLabel.Text);
                    string email = Session["email"].ToString();
                    int course_ID = Convert.ToInt32(Request.QueryString["courseID"]);
                    cmd.Parameters.Add(new SqlParameter("@course_ID", course_ID));
                    cmd.Parameters.Add(new SqlParameter("@myemail", email));
                    cmd.Parameters.Add(new SqlParameter("@post_ID", post_ID));
                    conn.Open();
                    cmd.ExecuteNonQuery();
                    conn.Close();
                    DataList2.DataBind();
                }
            }
Samer El Gendy
  • 1,683
  • 2
  • 23
  • 45

3 Answers3

2

Take DataList2.DataBind(); out of the foreach loop

            foreach (DataListItem item in DataList2.Items)
            {
                Label post_IDLabel = (Label)item.FindControl("post_IDLabel");
                string connStr = ConfigurationManager.ConnectionStrings["MyDbConn"].ToString();
                SqlConnection conn = new SqlConnection(connStr);
                SqlCommand cmd = new SqlCommand("delete_post", conn);
                cmd.CommandType = CommandType.StoredProcedure;
                int post_ID = Convert.ToInt32(post_IDLabel.Text);
                string email = Session["email"].ToString();
                int course_ID = Convert.ToInt32(Request.QueryString["courseID"]);
                cmd.Parameters.Add(new SqlParameter("@course_ID", course_ID));
                cmd.Parameters.Add(new SqlParameter("@myemail", email));
                cmd.Parameters.Add(new SqlParameter("@post_ID", post_ID));
                conn.Open();
                cmd.ExecuteNonQuery();
                conn.Close();
            }
            DataList2.DataBind();
Joe
  • 80,724
  • 18
  • 127
  • 145
2

You may not modify the collection while you are enumerating it. DataList2.DataBind modifies DataList2.Items, which is not allowed.

If you move DataBind outside the loop, it should work.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
1

I guess your problem is solved when you move the DataList2.DataBind out of the foreach.

But I think there is more wrong with your code, you should try to avoid calling the database from a loop. You should try to refactor this code so that you only make one call to the database. For example pass all post ID's in one parameter. Or maybe only the course_id and email if it is sufficient.

Tim Cools
  • 1,162
  • 10
  • 19
  • thanks a lot Tim, it worked but I didn't get the part about the post ID? could you explain more please? :) – Samer El Gendy Dec 29 '11 at 21:15
  • good question :-) you can find more information here: http://stackoverflow.com/questions/43249/t-sql-stored-procedure-that-accepts-multiple-id-values – Tim Cools Dec 29 '11 at 21:19
  • the idee is that you pass in the id's as a list to the stored procedure – Tim Cools Dec 29 '11 at 21:19
  • I go what you mean, however I don't want to delete all the values, just the selected item, do you have any idea how to do that? – Samer El Gendy Dec 29 '11 at 21:34
  • 1
    Now I see that you only want to delete a sinlge item. If you want to delete a single item you should not perform a loop. Instead you should use the CommandArgument to retrieve the id. So you should remove the loop, and use something like this to retrieve the id: int post_ID = (int) e.CommandArgument; – Tim Cools Dec 29 '11 at 21:57
  • I added the solution to your other question. – Tim Cools Dec 29 '11 at 22:00
  • thanks @Tim, however when i used it, i got nullexception, i guess i have done something wrong with the command – Samer El Gendy Dec 29 '11 at 22:46
  • if you still have problems with the deletion, please use the original question to comment, or create a new question, because this is not releated anymore to the question above. – Tim Cools Dec 30 '11 at 09:55