views:

735

answers:

7

Why is list1Instance and p lists in the Main method of the below code pointing to the same collection?

class Person
    {
        public string FirstName = string.Empty;
        public string LastName = string.Empty;

        public Person(string firstName, string lastName) {
            this.FirstName = firstName;
            this.LastName = lastName;
        }
    }

    class List1
    {
        public List<Person> l1 = new List<Person>();

        public List1()
        {
            l1.Add(new Person("f1","l1"));
            l1.Add(new Person("f2", "l2"));
            l1.Add(new Person("f3", "l3"));
            l1.Add(new Person("f4", "l4"));
            l1.Add(new Person("f5", "l5"));
        }
        public IEnumerable<Person> Get()
        {
            foreach (Person p in l1)
            {
                yield return p;
            }

            //return l1.AsReadOnly(); 
        }

    }  

    class Program
    {

        static void Main(string[] args)
        {
            List1 list1Instance = new List1();

            List<Person> p = new List<Person>(list1Instance.Get());           

            UpdatePersons(p);

            bool sameFirstName = (list1Instance.l1[0].FirstName == p[0].FirstName);
        }

        private static void UpdatePersons(List<Person> list)
        {
            list[0].FirstName = "uf1";
        }
    }

Can we change this behavior with out changing the return type of List1.Get()?

Thanks

+11  A: 

In fact, IEnumerable<T> is already readonly. It means you cannot alter any items in the underlying collection. You cannot alter the references to the Person type that are held in the collection. The return type is not read only, however, and since it's a reference type (i.e. a class), you can alter the original values through its reference. You should either use a struct as the return type (that makes a copy of the value each time it's returned, so the original value will not be altered, which can be costly, by the way) or use read only properties on the Person type to accomplish this task.

Mehrdad Afshari
+2  A: 

They aren't pointing to the same .Net collection, but rather, to the same Person objects. The line:

List<Person> p = new List<Person>(list1Instance.Get());

copies all the Person elements from list1Instance.Get() to list p. The word "copies" here means copies the references. So, your list and IEnumerable just happen to point to the same Person objects.

IEnumerable is always readonly, by definition. The objects inside are not.

siz
+4  A: 

Return a new instance of Person that is a copy of p instead of p itself in Get(). You'll need a method to make a deep-copy of a Person object to do this. This won't make them read only, but they will be different than those in the original list.

    public IEnumerable<Person> Get()
    {
        foreach (Person p in l1)
        {
            yield return p.Clone();
        }
    }
tvanfosson
+1  A: 

IEnumerable<T> is readonly

p is a new collection which doesn't depend on list1instance. The mistake you made, is that you thought that this line list[0].FirstName = "uf1";
would only modify one of the lists, when on fact you're modifying the Person object.
The two collections are distinct, they just happen to have the same items.
To prove that they are different, try adding and removing items from one of the lists, and you'll see that the other one isn't affected.

DonkeyMaster
A: 

First of all, your List in your class is public, so there's nothing stopping anyone from directly accessing the list itself.

Secondly, I would implement IEnumerable and return this in my GetEnumerator Method

return l1.AsReadOnly().GetEnumerator();
BFree
+1  A: 

You could make a deepclone of each item in the list, and never return references to your original items.

public IEnumerable<Person> Get()
{
  return l1
    .Select(p => new Person(){
      FirstName = p.FirstName,
      LastName = p.LastName
    });
}
David B
A: 

If your person object is a real object then you should consider using an immutable version.

 public class Person
 {
     public FirstName {get; private set;}
     public LastName {get; private set;}
     public Person(firstName, lastName)
     {
         FirstName = firstName;
         LastName = lastName;
     }
  }

In this way its not possible to change the content of the instance once created and therefore it isn't important that existing instances are reused in multiple lists.

AnthonyWJones