C# - Accessing Lists from another Class or copying it? - Performance & Best Practice

Question

I have a general question, concerning performance and best practice.

When working with a List (or any other datatype) from a different Class, which is better practice? Copying it at the beginning, working with the local and then re-copying it to the original, or always access the original?

An Example:

access the original:

        public class A
        {
            public static List<int> list = new List<int>();
        }

        public class B
        {

            public static void insertString(int i)
            {
                // insert at right place
                int count = A.list.Count;
                if (count == 0)
                {
                    A.list.Add(i);
                }
                else
                {
                    for (int j = 0; j < count; j++)
                    {
                        if (A.list[j] >= i)
                        {
                            A.list.Insert(j, i);
                            break;
                        }
                        if (j == count - 1)
                        {
                            A.list.Add(i);
                        }
                    }
                }
            }
        }

As you see I access the original List A.list several times. Here the alternative:

Copying:

        public class A
        {
            public static List<int> list = new List<int>();
        }

        public class B
        {

            public static void insertString(int i)
            {

                List<int> localList = A.list;

                // insert at right place
                int count = localList.Count;
                if (count == 0)
                {
                    localList.Add(i);
                }
                else
                {
                    for (int j = 0; j < count; j++)
                    {
                        if (localList[j] >= i)
                        {
                            localList.Insert(j, i);
                            break;
                        }
                        if (j == count - 1)
                        {
                            localList.Add(i);
                        }
                    }
                }

                A.list = localList; 
            }
        }

Here I access the the list in the other class only twice (getting it at the beginning and setting it at the end). Which would be better.

Please note that this is a general question and that the algorithm is only an example.

Answer 1

I won't bother thinking about performance here and instead focus on best practice:

Giving out the whole List violates encapsulation. B can modify the List and all its elements without A noticing (This is not a problem if A never uses the List itself but then A wouldn't even need to store it).

A simple example: A creates the List and immediately adds one element. Subsequently, A never bothers to check List.Count, because it knows that the List cannot be empty. Now B comes along and empties the List...

So any time B is changed, you need to also check A to see if all the assumptions of A are still correct. This is enough of a headache if you have full control over the code. If another programmer uses your class A, he may do something unexpected with the List and never check if that's ok.

---

Solution(s):

• If B only needs to iterate over the elements, write an IEnumerable accessor. If B mustn't modify the elements, make the accessor deliver copies.

• If B needs to modify the List (add/remove elements), either give B a copy of the List (containing copies of the elements if they needn't be modified) and accept a new List from B or use an accessor as before and implement the necessary List operations. In both cases, A will know if B modifies the List and can react accordingly.

Example:

class A
{
  private List<ItemType> internalList;

  public IEnumerable<ItemType> Items()
  {
    foreach (var item in internalList)
      yield return item;
      // or maybe  item.Copy();
      //           new ItemType(item);
      // depending on ItemType
  }

  public void RemoveFromList(ItemType toRemove)
  {
    internalList.Remove(toRemove);
    // do other things necessary to keep A in a consistent state
  }
}