c# list overwrite issue

Question

I have some code shown below that writes the values of a gridview to a list as shown. The code does get the correct values but when the second row of the grid is added to the list it overwrites the first row of the list.
Anybody know why this is happening?

C# Code

 List<Item> lstNewItems = new List<Item>(); // Control Items  
 lstNewItems.Clear();
 Item NewItem = new Item();
 foreach (GridViewRow PendingItemUnderControl in GridViewPendingList.Rows)
 {
    NewItem.Paramater = PendingItemUnderControl.Cells[0].Text.ToLower();
    NewItem.Type = (String)Session["BrowseType"];
    lstNewItems.Add(NewItem);
 }

score 7 · Accepted Answer · answered Dec 01 '12 at 12:09

You are creating one instance of Item class. It happens before your loop, so in reality you are working on the same object. You need to create new instance inside loop (in each iteration):

List<Item> lstNewItems = new List<Item>(); // Control Items  
lstNewItems.Clear();

foreach (GridViewRow PendingItemUnderControl in GridViewPendingList.Rows)
{
    Item NewItem = new Item();
    NewItem.Paramater = PendingItemUnderControl.Cells[0].Text.ToLower();
    NewItem.Type = (String)Session["BrowseType"];
    lstNewItems.Add(NewItem);
}

Sergey Kalinichenko · Answer 2 · 2012-12-01T12:16:06.053

This is because you keep adding the same object, and you mutate it in each iteration of the loop.

You allocate NewItem only once, then you configure and add it to the list. When the item in the list, however, you reconfigure it and add it to the list the second time. Now you have two items configured as the last item. Then you configure it as the third item, and add it again. All three items are configured the same, though: the previous two parameters and types are gone. Once the loop finishes, you have N copies of the same last item in your list.

Move the new inside the loop to fix the problem:

foreach (GridViewRow PendingItemUnderControl in GridViewPendingList.Rows)
{
    Item NewItem = new Item();
    ....
}

Better yet, make your Item immutable, and pass it the parameters in the constructor:

foreach (GridViewRow PendingItemUnderControl in GridViewPendingList.Rows)
{
    lstNewItems.Add(new Item(
        PendingItemUnderControl.Cells[0].Text.ToLower()
    ,   (String)Session["BrowseType"])
    );
}

score 1 · Answer 3 · answered Dec 01 '12 at 12:09

You are creating the new Item outside your loop, so each time you are updating the same element.

foreach (GridViewRow PendingItemUnderControl in GridViewPendingList.Rows)
{
    Item NewItem = new Item(); <------
    NewItem.Paramater = PendingItemUnderControl.Cells[0].Text.ToLower();
    NewItem.Type = (String)Session["BrowseType"];
    lstNewItems.Add(NewItem);

}

Move the new into the loop

c# list overwrite issue

3 Answers3

Linked