none
Modifying child list property sets all properties to the same value RRS feed

  • Question

  • Issue: When a list is created from reading from a table, and an entity from that list is passed by reference to a function that modifies it, every entity in the original list is modified.

    The following code illustrates the problem.  On my system, the GetPeopleFromDatabase method returns a list of 32 employees.  Only one is named Sam.

    After passing Sam by reference to AddPets, all 32 entities in the people list have a pet.

    GetPeopleFromMemory works as expected.

    using
     System;
    using
     System.Collections.Generic;
    using
     System.Linq;
    using
     System.Web;
    using
     System.Web.UI;
    using
     System.Web.UI.WebControls;
    
    public
     partial
     class
     WebServiceTest : System.Web.UI.Page
    {
     protected
     void
     Page_Load(object
     sender, EventArgs e)
     {
    
     }
    
     protected
     void
     btnTest_Click(object
     sender, EventArgs e)
     {
      List<Person> people = GetPeopleFromDatabase();
      List<Person> employeesNamedSam = people.Where(x => x.Name.Contains("Sam"
    )).ToList();
      AddPets(ref
     employeesNamedSam);
      // At this point EVERY entity in the people list has a pet. Only Sam should have a pet!!
    
    
    
      List<Person> people2 = GetPeopleFromMemory();
      List<Person> funnyGuys = people2.Where(x => x.Name.Contains("Larry"
    )).ToList();
      AddPets(ref
     funnyGuys);
      // This list works as expected. Only Larry has a pet.
    
      
     }
    
     protected
     List<Person> GetPeopleFromDatabase()
     {
      Context db = new
     Context();
      List<Person> people = (from
     empl in
     db.Employees
            select
     new
     Person
            {
            ID = empl.EmployeeID,
            Name = empl.FirstName,
            Pets = new
     List<Pet>()
            }).ToList();
      
      return
     people;
     }
    
     protected
     List<Person> GetPeopleFromMemory()
     {
      List<Person> people = new
     List<Person>();
      people.Add(new
     Person { ID = 1, Name = "Larry"
    , Pets = new
     List<Pet>() });
      people.Add(new
     Person { ID = 2, Name = "Moe"
    , Pets = new
     List<Pet>() });
      people.Add(new
     Person { ID = 3, Name = "Curly"
    , Pets = new
     List<Pet>() });
      return
     people;
     }
    
    
     protected
     void
     AddPets(ref
     List<Person> people)
     {
      foreach
     (Person person in
     people)
       person.Pets.Add(new
     Pet { ID = 1, Name = "Fido"
     });
     }
    
    
     protected
     class
     Person
     {
      public
     long
     ID;
      public
     string
     Name;
      public
     List<Pet> Pets;
     }
    
     protected
     class
     Pet
     {
      public
     long
     ID;
      public
     string
     Name;
     }
    
    
     protected
     void
     btnTest2_Click(object
     sender, EventArgs e)
     {
    
     }
    
    }
    
    

    • Edited by Sam2 Thursday, April 22, 2010 4:02 PM
    Wednesday, April 21, 2010 5:25 PM

Answers

  • I can't really explain this, but I have replicated your problem and found a couple of ways of making it work.

    It seems that the List<Pets> assigned to each new Person in GetPeopleFromDatabase is the same list (i.e. it's not a new list for each new person). Hence when you add a pet to the list, all people get it. This doesn't happen in the GetPeopleFromMemory because each person is created on a separate code line.

    The first, and I think best, way around this is to use a proper constructor for your new Person in which you create the new List<Pet>, thus:

     public Person(long id, string name)
     {
      ID = id;
      Name = name;
      Pets = new List<Pet>();  // <---
     }

    and then your GetPeopleFromDatabase looks like this:

     protected List<Person> GetPeopleFromDatabase()
     {
      Context db = new Context();
      List<Person> people = (from empl in db.Employees
            select new Person (empl.EmployeeID,empl.FirstName)).ToList();
      
      return people;
     }
    

    This means that each person has an empty List<Pets>.

    The other way is to drop the "Pets = new List<Pet>()" from the creation of the new person which means each person's Pets == null. In AddPets you would have to have a conditional check if PEts == null then Pets = new List<Pet>().

    Does this help?

    Wednesday, April 28, 2010 8:36 AM

All replies

  • Update:  Apparently this has nothing to do with passing by reference.  I'm trying to find a fix and I observed this dosn't work either:

     

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Web;
    using System.Web.UI;
    using System.Web.UI.WebControls;
    
    public partial class WebServiceTest : System.Web.UI.Page
    {
      protected void Page_Load(object sender, EventArgs e)
      {
    
      }
    
      protected void btnTest_Click(object sender, EventArgs e)
      {
        List<Person> people = GetPeopleFromDatabase();
        List<Person> employeesNamedSam = people.Where(x => x.Name.Contains("Sam")).ToList();
        //AddPets(ref employeesNamedSam);
        employeesNamedSam = AddPets2(employeesNamedSam);
        // At this point EVERY entity in the people list has a pet. Only Sam should have a pet!!
    
    
        List<Person> people2 = GetPeopleFromMemory();
        List<Person> funnyGuys = people2.Where(x => x.Name.Contains("Larry")).ToList();
        AddPets(ref funnyGuys);
        // This list works as expected. Only Larry has a pet.
        
      }
    
      protected List<Person> GetPeopleFromDatabase()
      {
        Context db = new Context();
        List<Person> people = (from empl in db.Employees
                   select new Person
                   {
                    ID = empl.EmployeeID,
                    Name = empl.FirstName,
                    Pets = new List<Pet>()
                   }).ToList();
        
        return people;
      }
    
      protected List<Person> GetPeopleFromMemory()
      {
        List<Person> people = new List<Person>();
        people.Add(new Person { ID = 1, Name = "Larry", Pets = new List<Pet>() });
        people.Add(new Person { ID = 2, Name = "Moe", Pets = new List<Pet>() });
        people.Add(new Person { ID = 3, Name = "Curly", Pets = new List<Pet>() });
        return people;
      }
    
    
      protected void AddPets(ref List<Person> people)
      {
        foreach (Person person in people)
          person.Pets.Add(new Pet { ID = 1, Name = "Fido" });
      }
    
      protected List<Person> AddPets2(List<Person> people)
      {
        foreach (Person person in people)
          person.Pets.Add(new Pet { ID = 1, Name = "Fido" });
        return people;
      }
    
    
    
      protected class Person
      {
        public long ID;
        public string Name;
        public List<Pet> Pets;
      }
    
      protected class Pet
      {
        public long ID;
        public string Name;
      }
    
    
      protected void btnTest2_Click(object sender, EventArgs e)
      {
    
      }
    
    }
    

    Wednesday, April 21, 2010 6:14 PM
  • Still trying to fix this and its only getting worse:

     

    protected void btnTest_Click(object sender, EventArgs e)
      {
        List<Person> people = GetPeopleFromDatabase();
        people[3].Pets.Add(new Pet { ID = 9, Name = "Gomer" }); // Adds a pet to EVERY row on the people list.

     

    I am totally confused.  Apparently I'm missing something that should be completely obvious or this this is just plain broken.

    Wednesday, April 21, 2010 8:35 PM
  • Microsoft please assist me with this.

    Thank you,

    Sam

    Thursday, April 22, 2010 2:57 PM
  • Microsoft please assist me with this.

    Thank you,

    Sam

    Friday, April 23, 2010 2:59 PM
  • Microsoft please assist me with this.

    Thank you,

    Sam

    Monday, April 26, 2010 2:53 PM
  • Did I say something wrong?  Am I posting in the wrong forum?

    Is anyone here?  The example I wrote up is trivial but I have a lot of code that is designed around the way this is supposed to work and it is broken.

    Thanks,

    Sam

    Tuesday, April 27, 2010 3:00 PM
  • You are posting to the right forum, but I guess that no-one has been able to identify the problem. Me neither, I'm afraid! It all looks ok.

    The GetPeopleFromDatabase method looks ok, though you could put the 'where' clause in there:

     protected List<Person> GetPeopleFromDatabase()
     {
      Context db = new Context();
      List<Person> people = (from empl in db.Employees
            where empl.Name.Contains("Sam")
            select new Person
            {
            ID = empl.EmployeeID,
            Name = empl.FirstName,
            Pets = new List<Pet>()
            }).ToList();
      
      return people;
     }

    But if that's not what you want to do, then I'm stumped. I am guessing that you have tried all sorts of outputting to prove what is in the employeesNamedSam list before calling AddPet2 - i.e. if the Where method has done it's job - because it seems like it is returning the full list of employees rather than just Sam.  'people' and 'employeesNamedSam' are both lists so you should be able to do a .Count() on them.

    Tuesday, April 27, 2010 9:49 PM
  • Hi John, Thank you for your reply.

    Your comment about the where clause is spot on but I modeled this test after a much larger program in an effort to debug it so I am doing some things that are odd.

    Here is what the problem is:

    I create a list called people by reading a table from disk.  On my system the list is populated with 32 entities.

    I create a second list by selecting against the people list (employessNamedSam).   On my system the second list is populated with one entity.

    The second list (containing a single entity) is passed to AddPets().  On my system I step through it with the debugger and I observe it loop once.

    At this point you would expect to have a list (people), and each row in the people list should have an empty pet property. The excepton(s) should be any employee named Sam, and those entities pet property should contain one pet entity. On my system there is one employee named Sam.

    Here is the issue - After the call to AddPets, every entity in the people list has a pet entity with one pet.  That is incorrect - only employees named sam should have pets.

     

     

     

     

     

     

     

    Tuesday, April 27, 2010 11:34 PM
  • I can't really explain this, but I have replicated your problem and found a couple of ways of making it work.

    It seems that the List<Pets> assigned to each new Person in GetPeopleFromDatabase is the same list (i.e. it's not a new list for each new person). Hence when you add a pet to the list, all people get it. This doesn't happen in the GetPeopleFromMemory because each person is created on a separate code line.

    The first, and I think best, way around this is to use a proper constructor for your new Person in which you create the new List<Pet>, thus:

     public Person(long id, string name)
     {
      ID = id;
      Name = name;
      Pets = new List<Pet>();  // <---
     }

    and then your GetPeopleFromDatabase looks like this:

     protected List<Person> GetPeopleFromDatabase()
     {
      Context db = new Context();
      List<Person> people = (from empl in db.Employees
            select new Person (empl.EmployeeID,empl.FirstName)).ToList();
      
      return people;
     }
    

    This means that each person has an empty List<Pets>.

    The other way is to drop the "Pets = new List<Pet>()" from the creation of the new person which means each person's Pets == null. In AddPets you would have to have a conditional check if PEts == null then Pets = new List<Pet>().

    Does this help?

    Wednesday, April 28, 2010 8:36 AM
  • Thank you for your answer John.  It is not what I wanted to hear.

    I think it would be helpful to hear from Microsoft as to whether this is a bug and what the intended resolution is.  It certainly has caused me hours of grief and I have a huge workload ahead of me as I have used this pattern extensively.

     

    Wednesday, April 28, 2010 3:27 PM