CODEWITHSUNDEEP
c#
vincentsty
vincentsty

Reputation: 3221

Modifying list inside list in C#

I am trying to modify the value inside the inner list by iterating the list. However, i keep getting the same outcome for the inner list irrespective of the outer list. I expect the outcome of the maxSpeed is different for different vehicle.

Hope someone could help on this.

Note that this is not a random number generator problem. This is just a sample code that I produce and this issue do exist in my project code without the use of random number generator.

Run C# code here

List<Vehicle> mainList = new List<Vehicle>();
List<Properties> defaultPropertyList = new List<Properties>{
    new Properties() { maxSpeed = 0, isTwoDoor = true },
    new Properties() { maxSpeed = 0, isTwoDoor = true },
};


mainList.Add(
    new Vehicle() {
        number = 1,
        property = new List<Properties>(defaultPropertyList)
    }
);
mainList.Add(
    new Vehicle() {
        number = 2,
        property = new List<Properties>(defaultPropertyList)
    }
);           

foreach(Vehicle vehicle in mainList) {
    Random rnd = new Random();
    vehicle.property.ForEach(x => x.maxSpeed =  rnd.Next(1, 100));
}

Upvotes: 0

Views: 102

Answers (2)

Scott Hannen
Scott Hannen

Reputation: 29222

defaultPropertyList is just one list containing one set of items. Each Vehicle has the same instance of List<Properties>. There's not one per Vehicle. There's just one, and they're all sharing it. That's why no matter how you change the properties, they all have the same properties.

To fix it, don't create one and share it. Just create as many as you need. Maybe this isn't you, but when I started off I was afraid of creating lots of objects and thought I could optimize by creating as few as possible.

While it's true that we don't want to unnecessarily create lots of expensive objects, there's no need to be stingy about it. Create as many as you need. For example, in a web application it's impossible to even keep track of how many objects get created just to respond to a single request.

You could just do this:

Random rnd = new Random(); // Despite what I just said,
                           // you only need one of these this time.
foreach(Vehicle vehicle in mainList) {        
    vehicle.property = new List<Properties>{
        new Properties() { maxSpeed = rnd.Next(1, 100), isTwoDoor = true },
        new Properties() { maxSpeed = rnd.Next(1, 100), isTwoDoor = true },
}

Upvotes: 1

Yaakov Ellis
Yaakov Ellis

Reputation: 41510

The problem is that when you initialize the property field for each vehicle, you are adding references to the same two Properties objects (defined at the top). So they exist in all vehicles, and when you modify one, it is modified in both places (since it is the same reference).

You will want to instead make copies of the objects in the defaultPropertyList when you initialize a new vehicle. Then they will exist independently.

One approach: Make a new constructor for Vehicle that takes in the default properties, and copy them there.

public Vehicle(List<Properties> defaultProps) 
{
  property = new List<Properties>();
  foreach (var p in defaultProps)
  {
    var newProp = new Properties {
                                    maxSpeed = p.maxSpeed,
                                    isTwoDoor = p.isTwoDoor
                                    // add any other properties here
                                 };
    property.Add(newProp);
  }
}

Upvotes: 5

Related Questions