4

I have two lists, the original one and a copied one. I made a copy of the original list, because of one reason:

I need to process/work data from the original list, but I shouldn't edit it. So I created a copy of that original list to work with. But somehow changes I do on the copied list still modifies the original.

Here is my code:

forPrintKitchenOrders = new List<OrderTemp>();
foreach (var itemss in forPrintKitchen)
{
  forPrintKitchenOrders.Add(itemss); // HERE I AM ADDING ITEMS TO ANOTHER LIST BECAUSE I DON'T WANT TO EDIT ORIGINAL LIST (Change quantity etc)
}

if (forPrintKitchen.Count > 0)
{

  foreach (var item in forPrintKitchenOrders.ToList())
  {
      foreach (var item2 in mainList)
      {
          if (item2.MainProductID == Convert.ToInt32(item._articleCode))
          {
              //I don't know why this is happening. I loop another list (copy of original list, because I didn't want to harm original list), and when I find certain item I am reducing quantity (-1),
              //And later I realized and saw while I was debugging, that the value of quantity in my original "forPrintKitchen" list is also edited, I don't know how changed reflected there..

              int calculate = Convert.ToInt32(item._quantity)-1; //this block is making me trouble, here I am reducing quantity and later that reflects to my forPrintKitchen list even if I am editing and looping //forPrintKitchenOrders(original's copy)
              item._quantity = calculate.ToString();
          }
      }
  }
    foreach (var items in forPrintKitchen) //THIS IS MY ORIGILAN LIST AND SHE SHOULD NOT BE EDITED WHEN I EDIT "forPrintKitchenOrders" item
    {
    //Original List
        OrdersKitchen kitchen = new OrdersKitchen();
        kitchen.ProductID = Convert.ToInt32(items._articleCode);
        kitchen.UserID = Util.User.UserID;
        kitchen.UserName = Util.User.FirstName
        kitchen.LastName = Util.User.LastName
        kitchen.BillID = bill.BillID;
        kitchen.Quantity = Convert.ToInt32(items._Quantity);
        OrdersKitchen.Add(kitchen);
    }
    foreach (var itemss in mainList)
    {

        OrdersKitchen kitchen2 = new OrdersKitchen();
        kitchen2.ProductID = Convert.ToInt32(itemss.MainProductID);
        kitchen2.UserID = User.UserID;
        kitchen2.UserName = Util.User.FirstName;
        kitchen2.LastName = Util.User.LastName;
        kitchen2.BillID = bill.BillID;
        kitchen2.Quantity = Convert.ToInt32(0); //HARDCODE ZERO
        OrdersKitchen.Add(kitchen2);
    }
 }

mainList.Clear();
//forPrintKitchenOrders.Clear();
}

After I saw the replies, I read @sachin's post and wrote a code similar to theirs. Is this alright? It looks like it's working right now, but I am not sure is this solution ok?

foreach (var itemss in forPrintKitchenOrders)
{
    forPrintKitchenOrders.Add(new OrderTemp(itemss._articleCode,itemss._name,itemss._quantity,itemss._amount));
}

public class OrderTemp
{
        public string _articleCode;
        public string _name;
        public string _quantity;
        public double _amount;

        public OrderTemp(string articleCode, string name, string quantity, double amount)
        {
            _articleCode = amount;
            _name = name;
            _quantity = quantity;
            _amount = amount;
        }
}
BDL
  • 21,052
  • 22
  • 49
  • 55
Roxy'Pro
  • 4,216
  • 9
  • 40
  • 102
  • 4
    These are two lists referencing the same objects. You create a new **set** that references the same objects of the old **set**. If you change a property of an object in the first, then it is changed in the second. This is because the objects you're using are reference types and not value types. – Zein Makki Sep 22 '16 at 07:42
  • 1
    See here: http://stackoverflow.com/q/14007405/1136211 – Clemens Sep 22 '16 at 07:44
  • 1
    As @user3185569 comment on the first post, you are NOT coping the object from one list to another. You are coping THE REFERENCE. Yo have to change `forPrintKitchenOrders.Add(itemss);` to`forPrintKitchenOrders.Add(itemss.Clone());` – Rumpelstinsk Sep 22 '16 at 07:48

3 Answers3

5

The collection that you are referring to as 'Copy List' is not actually a Copy.

The elements inside the Collection are referring to the same Objects as in the Original Collection.

You will have to replace your copying foreach loop with something like this:

foreach (var item in forPrintKitchen)
{
    forPrintKitchenOrders.Add(item.Clone()); // The clone method should return a new Instance after copying properties from item.
}

The Clone method should create new Instance and copy each of the properties from the instance being cloned and then return the newly created Instance.

Basically you'll have to define a method named Clone (name doesn't matter) in OrderTemp class like this:

public class OrderTemp
{
    /*  other members of the class */
    public OrderTemp Clone()
    {
        return new OrderTemp
        {
            property1 = this.property1;
            property2 = this.property2;
            /* and so on */
        };
    }
}
sachin
  • 2,341
  • 12
  • 24
  • By following your post I will edit my question and post solution I've found, but I am not sure is it right way. so could you take a look – Roxy'Pro Sep 22 '16 at 08:33
  • Could you answer me is that what I wrote in my post under EDIT section OK? – Roxy'Pro Sep 22 '16 at 21:13
  • and can you give me little bit explanation why does it work acctualy? :) thanks – Roxy'Pro Sep 23 '16 at 06:40
  • @Roxy'Pro The `new` operator is the Key here. When you are creating an Instance using new operator, you're creating an instance different from the original one. – sachin Sep 23 '16 at 06:46
2

You basicaly create shallow copy. It means it will copy all simple types, like references and generic types, but NOT objects content.

As solution you need to totaly copy your objects:

foreach (var itemss in forPrintKitchen)
{
  forPrintKitchenOrders.Add(new OrderTemp(){ /*copy itemss here*/ });
}

I love to use AutoMapper for this task, but if you don't want to take framework in you project you can just implement IClonable on your OrderTemp type and all necessary nested types and call Clone() method.

eocron
  • 6,885
  • 1
  • 21
  • 50
0

You made a copy of the list, but the copy still contains references to the same objects; the objects themselves are not copied.

Thomas Levesque
  • 286,951
  • 70
  • 623
  • 758