1

I have constructed the following simplistic example to illustrate the problem I'm having in my actual code, which is much more complex.

First, my simple classes:

public class InnerTemp {
   public int A { get; set; }
   public int B { get; set; }
   public int C { get; set; }
}

public class OuterTemp {
   public List<InnerTemp> InnerTemps { get; set; }
}

And the implementation:

var outerTemps = new List<OuterTemp>();
var innerTemps = new List<InnerTemp>();
innerTemps.Add(new InnerTemp() { A = 1, B = 2, C = 3 });
innerTemps.Add(new InnerTemp() { A = 4, B = 5, C = 6 });

for( int i = 1; i < 5; i++ ) {
      foreach( InnerTemp innerTemp in innerTemps ) {
           innerTemp.A += i;
           innerTemp.B += i;
           innerTemp.C += i;
       }

       var outerTemp = new OuterTemp()
       {
           InnerTemps = innerTemps
       };

       outerTemps.Add(outerTemp);
}

I set my breakpoint on the last line, the closing curly brace. On the first iteration, the value of outerTemps[0].InnerTemps[0].A is 2, as expected. But on the 2nd loop, the value of outerTemps[0].InnerTemps[0].A becomes 4, which is baffling me. The first object's property A is being stomped merely because another object was added. I fully expect outerTemps[1].InnerTemps[0].A to be different, but why is my original property stomped?

How can I change this so that previous object properties don't get overwritten each time a new object is added to the collection?

HerrimanCoder
  • 6,835
  • 24
  • 78
  • 158
  • What would you want `outerTemps[0].InnerTemps[0].A` to be on the second loop? – IS4 Feb 09 '17 at 01:11
  • 2
    You should take a look at [shallow vs deep copy](http://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy). You are copying a reference to `innerTemps` instead of doing a deep copy of all the elements. This causes the same InnertTemp reference to be modified for each outerTemp object. – E. Moffat Feb 09 '17 at 01:11
  • Its obvious that you are modifying the same list in each of the iteration. You need to create a new list of InnerTemp with deep copy of all the objects in it on each iteration of outer for loop. – Chetan Feb 09 '17 at 01:27

1 Answers1

2

As I mentioned in my comment you are creating a shallow copy of the innerTemps list which causes the references to be copied instead of creating deep copies of each InnerTemp object in the list.

If you want to have separate InnerTemp objects in the newly created OuterTemp that are tracked and modified separately, you have to create a deep copy of them.

Per the advice here I would recommend creating a Copy method on your InnerTemp class:

public class InnerTemp
{
   public int A { get; set; }
   public int B { get; set; }
   public int C { get; set; }

   public static InnerTemp Copy(InnerTemp source)
   {
      return new InnerTemp { A = source.A, B = source.B, C = source.C };
   }
}

And use that when creating the new list for each OuterTemp:

var outerTemp = new OuterTemp()
{
    InnerTemps = innerTemps.Select(InnerTemp.Copy).ToList();
};

This will prevent the values in the outerTemps list from being modified by the loop counter. Note that the .ToList() implicitly creates a deep copy by evaluating the Select enumerable and returning a new list, so you don't have to worry about the list references causing problems.

Community
  • 1
  • 1
E. Moffat
  • 3,165
  • 1
  • 21
  • 34