JavaScriptShadow

CA1002; Some developers would rather just return a List<T>. In a nutshell: return Collection<T> from any public method if you are returning a collection (little c) of things!

But, List<T> works faster… and it has methods like Sort(), and Find()!

If you are reading this, by now you have probably read Stackoverflow articles such as this one:
C#: Difference between List<T> and Collection<T> (CA1002, Do not expose generic lists)

I am not going to argue the case of Collection<T> vs. List<T>. I want to find a practical way to code better and not get the warning without suppressing it. Consider this case:

	public IEnumerable<int> MyMethod()
	{
		List<int>; myList = new List<int>()
		for (int (i) = 0; (i) <= 1000000; (i)++)
		{
			myList.Add(i);
		}

		return myList;
	}

Visual Studio Code Analysis will not give a CA1002 when we build this. The List<T> is scoped within the method and we are returning IEnumerable<T>.

For most shops where the code is known by all, and never used or looked at outside the shop, I guess the above would work? Seems plausible. But, we cannot enhance it later down the road if we needed to. Say, for example, we needed to raise an event that the list was changed somehow. As a developer friend used to say: “It makes me feel dirty.” Feelings aside, the recommendation is to return a Collection, as it is extensible and will not break any older code.

Personally, I don’t feel real good about leaving it like that. I feel the need to refactor that further and have it be more universal. From Martin Fowler’s “Refactoring” book, we learn that the main reasons to refactor is to make our code manageable/extensible, and readable. The exception would be to increase performance. So, do we have a performance issue here?

While the example above might be faster in execution, we tested it out with ticks, and for a million records the difference was 809000 ticks on average (returning Collection) vs. 799000 ticks on average (returning IEnumerable).

So, performance not really being an issue here, let’s say we follow good refactoring principles to make our code more readable, and maintainable/extensible.

The simplest solution:

	public Collection<int> MyMethod()
	{
		List<int> myList = new List<int>()
		for (int (i) = 0; (i) <= 1000000; (i)++)
		{
			myList.Add(i);
		}

		return new Collection<int>(myList);
	}

Perhaps for 99 percent(?) of what we do that would be ok. So, why are we doing this?

1. We aren’t getting a CA1002 warning.
2. We are returning a Collection, so if we decide to extend/enhance/soup up the collection, we will not be breaking any other calling code later.
3. We are not exposing extraneous methods associated with a List<T> (look up the methods in List vs Collection on MSDN)

More explanation:

Some of you will be asking why, or when do we need to soup up Collection<T>? How do we do that?

Some will ask how can we get cool methods List<T> has like Sort() or Find() in this “Collection”?

How can you do both with a Collection<T>? How can you write good, readable, maintainable code and add functionality like Sort() and Find() later, without breaking code that calls this?

Some of the methods in the System.Collections.ObjectModel.Collection<T> are virtual. So if we have our new Collection inherit from Collection, we can override those methods. We can also add our own methods and properties to our new Collection class if we wanted. Since such a class would inherit from Collection<T>, any existing code expecting a return type Collection<T> would not break and continue to operate, and newer code can take advantage of new features.

For example: Let us assume we want the code using our new Collection class to get notified when the Collection is changed. This seems like a more practical reason why we would want to extend the behavior of the .Net Collection class. A use case for this might be a data broker service where many different branch companies may be sending data in to be acted upon, and you need to audit the collection items as the collection changes, or your company is cyber security aware and implements layers of data security, validation, etc… in the UI, code, and database, and as the state of the collection changes, you need to ensure data is valid, not tampered with, audited, etc…

Looking at the source code for Collection we notice, that we can override the following in any custom class we create that inherits System.Collections.ObjectModel.Collection<T>:

• protected virtual void ClearItems()
• protected virtual void InsertItem(int index, T item)
• protected virtual void RemoveItem(int index)
• protected virtual void SetItem(int index, T item)

We also note that “items” in the Collection class is declared as:

IList<T>; items;

And the default constructor for a Collection class is:

public Collection() {
    items = new List<T>();
}

The Collection<T> class, internally by default uses a List. It is a wrapper class around a List, allowing us a clean way to use the framework to extend a list!

And the overridable methods listed above, all act on, you guessed it: the IList items!

The other constructor in the Collection<T> class is:

public Collection(IList<T> list) {
    if (list == null) {
        ThrowHelper.ThrowArgumentNullException(ExceptionArgument.list);
    }
    items = list;
}

We can conclude, that the Collection class will internally use anything that implements the IList interface. For simplicity, assume we have a Person:

namespace CollectionsExamples
{
    using System;

    public class Person
    {
        public string LastName { get; set; }

        public string FirstName { get; set; }

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

        public override string ToString()
        {
            return string.Format("Person: {0} LastName: {1}, FirstName: {2} {3}", "{", this.LastName, this.FirstName, "}");
        }
    }
}

And assume a class of People as follows:

namespace CollectionExample.Model
{
    using System;
    using System.Collections.Generic;

    public class People
    {
        private IList<Person> people;

        public People()
        {
            this.people = new List<Person>();
        }

        public People(IList<Person> list)
        {
            if (list == null)
            {
                throw new ArgumentNullException("list");
            }

            this.people = list;
        }

        public List<Person> RetrieveAllPeople()
        {
            if (this.people == null)
            {
                this.people = new List<Person>();
            }

            if(this.people.Count == 0)
            {
                this.people.Add(new Person("Simpson", "Homer"));
                this.people.Add(new Person("Bond", "James"));
            }

            return (List<Person>)this.people;
        }
    }
}

When we build this code we get the CA1002 warning:

This being our first version of the code we decide to return a collection, as follows:

        public Collection<Person> RetrieveAllPeople()
        {
            if (this.people == null)
            {
                this.people = new List<Person>();
            }

            if(this.people.Count == 0)
            {
                this.people.Add(new Person("Simpson", "Homer"));
                this.people.Add(new Person("Bond", "James"));
            }

            return new Collection<Person>(this.people);
        }

We do not get a CA1002 warning. Now we will use this in a program. I am curious if our collection is initializing it’s internal List<T> by reference or value? We will test that out in our program below:

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using CollectionExample.Model;

[assembly: CLSCompliant(true)]
namespace CollectionExample
{
    class Program
    {
        static void Main()
        {
            Collection<Person> moviePeople = new People().RetrieveAllPeople();

            Display(moviePeople);

            List<Person> otherPeople = new List<Person>();
            foreach(Person p in moviePeople)
            {
                otherPeople.Add(p);
            }

            Collection<Person> otherMoviePeople = new Collection<Person>(otherPeople);
            otherMoviePeople.Add(new Person("Underwood", "Frank"));

            Display(otherPeople);

            Console.Read();
        }

        private static void Display(IList<Person> people)
        {
            Console.WriteLine("{0} Displaying people:", Environment.NewLine);
            foreach(Person p in people)
            {
                Console.WriteLine("   - {0}", p.ToString());
            }

            Console.WriteLine(" ==== End of List ====");
        }
    }
}

We will now run this “Version 1.0” program:

Notice, in the second display that what you are seeing is the contents of the List<Person> otherPeople. This lets you know that when you create a new instance on a Collection<T> by passing a List<T> in the constructor, that it is done by reference.

Now, a new program has a need to know when the collection changes. We need a new “enhanced” collection that will raise some sort of “change” event the program can handle as it sees fit. The second requirement of this new program is that it needs to sort the collection after it is changed. To recap, two requirements:

1. Fire a change event when the collection is changed.
2. Sort the collection.

Let’s make some changes to our classes.

In preparation of being able to sort the collection of Person objects, let’s have the Person class implement IComparable, ignoring the overrides to the =, <, >, != for less code clutter in the example:

namespace CollectionExample.Model
{
    using System;

    public class Person : IComparable<Person>
    {
        public string LastName { get; set; }

        public string FirstName { get; set; }

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

        public override string ToString()
        {
            return string.Format("Person: {0} LastName: {1}, FirstName: {2} {3}", "{", this.LastName, this.FirstName, "}");
        }

        public int CompareTo(Person other)
        {
            int lastNameToOtherResult = this.LastName.CompareTo(other.LastName);
            int firstNameToOtherResult = this.FirstName.CompareTo(other.FirstName);

            if (lastNameToOtherResult != 0)
            {
                return lastNameToOtherResult;
            }

            if (firstNameToOtherResult != 0)
            {
                return firstNameToOtherResult;
            }

            return 0;
        }
    }
}

And a new class that inherits from Collection<T> named PersonCollection:

namespace CollectionExample.Model
{
    using System;
    using System.Collections.Generic;
    using System.Collections.ObjectModel;

    public class PersonCollection : Collection<Person>
    {
        public PersonCollection() : base()
        {
        }

        public PersonCollection(IList<Person> people)
            : base(people)
        {
        }

        public event EventHandler<PersonCollectionChangedEventArgs> Changed;

        public void Sort()
        {
            List<Person> items = (List<Person>)Items;
            items.Sort();
        }

        protected override void InsertItem(int index, Person item)
        {
            if (item == null)
            {
                throw new ArgumentNullException("item");
            }

            base.InsertItem(index, item);

            EventHandler<PersonCollectionChangedEventArgs> temp = this.Changed;
            if (temp != null)
            {
                temp(this, new PersonCollectionChangedEventArgs(CollectionOperation.Add, item.ToString(), null));
            }
        }

        protected override void SetItem(int index, Person item)
        {
            if (item == null)
            {
                throw new ArgumentNullException("item");
            }

            Person replaced = Items[index];
            base.SetItem(index, item);

            EventHandler<PersonCollectionChangedEventArgs> temp = this.Changed;
            if (temp != null)
            {
                temp(this, new PersonCollectionChangedEventArgs(CollectionOperation.Update, replaced.ToString(), item.ToString()));
            }
        }

        protected override void ClearItems()
        {
            base.ClearItems();

            EventHandler<PersonCollectionChangedEventArgs> temp = this.Changed;
            if (temp != null)
            {
                temp(this, new PersonCollectionChangedEventArgs(CollectionOperation.Clear, null, null));
            }
        }

        protected override void RemoveItem(int index)
        {
            Person removedItem = Items[index];
            base.RemoveItem(index);

            EventHandler<PersonCollectionChangedEventArgs> temp = this.Changed;
            if (temp != null)
            {
                temp(this, new PersonCollectionChangedEventArgs(CollectionOperation.Remove, removedItem.ToString(), null));
            }
        }
    }
}

We will need to add some additional classes to help out. An enum to define the type of operations on our collection, and some event arguments to pass when invoking an event when our collection changes. First the collection operation, then the change event arguments:

namespace CollectionExample.Model
{
    public enum CollectionOperation
    {
        /// <summary>
        /// An add item to collection operation.
        /// </summary>
        Add,

        /// <summary>
        /// A remove item from collection operation.
        /// </summary>
        Remove,

        /// <summary>
        /// Update an item in the collection operation.
        /// </summary>
        Update,

        /// <summary>
        /// Clear the collection operation.
        /// </summary>
        Clear
    }
}

And the event argument:

namespace CollectionExample.Model
{
    using System;

    public class PersonCollectionChangedEventArgs : EventArgs
    {
        private string item;
        private CollectionOperation changeType;
        private string replacement;

        public string Item
        {
            get { return item; }
        }

        public CollectionOperation ChangeType
        {
            get { return changeType; }
        }

        public string Replacement
        {
            get { return replacement; }
        }

        public PersonCollectionChangedEventArgs(CollectionOperation changeType, string item, string replacement)
        {
            this.item = item;
            this.changeType = changeType;
            this.replacement = replacement;
        }
    }
}

We make a small change to our people class, to return our enhanced PeopleCollection class:

        public PersonCollection RetrieveAllPeople()
        {
            if (this.people == null)
            {
                this.people = new List<Person>();
            }

            if(this.people.Count == 0)
            {
                this.people.Add(new Person("Simpson", "Homer"));
                this.people.Add(new Person("Bond", "James"));
            }

            return new PersonCollection(this.people);
        }

Now we add our second program that utilizes the new “enhanced” PersonCollection. Will we need to make any changes to the first program now that our library is updated? We run the first program as is with no changes and get:

Same results as the first time. And what about our new program? What can it do? Let’s look at the code:

using System;
using System.Collections.Generic;
using CollectionExample.Model;

[assembly: CLSCompliant(true)]
namespace PersonCollectionConsoleApplication
{
    class Program
    {
        static void Main()
        {
            PersonCollection moviePeople = new People().RetrieveAllPeople();
            moviePeople.Changed += People_Changed;

            Display(moviePeople);

            moviePeople.Add(new Person("Mouse", "Mickey"));
            moviePeople.Sort();

            Display(moviePeople);

            Console.Read();
        }

        protected static void People_Changed(object sender, PersonCollectionChangedEventArgs e)
        {
            Console.Write("{0}  -- Change Detected: ", Environment.NewLine);
            switch (e.ChangeType)
            {
                case CollectionOperation.Add:
                    Console.WriteLine("{0} was added.{1}", e.Item, Environment.NewLine);
                    break;
                case CollectionOperation.Remove:
                    Console.WriteLine("{0} was removed.{1}", e.Item, Environment.NewLine);
                    break;
                case CollectionOperation.Update:
                    Console.WriteLine("{0} was replaced with {1}.{2}", e.Item, e.Replacement, Environment.NewLine);
                    break;
                case CollectionOperation.Clear:
                    Console.WriteLine("people was cleared.{0}", Environment.NewLine);
                    break;
                default:
                    Console.WriteLine("Error: Unknown Operation.{0}", Environment.NewLine);
                    break;
            }
        }

        private static void Display(IList<Person> people)
        {
            Console.WriteLine("{0} Displaying PersonCollection people:", Environment.NewLine);
            foreach (Person p in people)
            {
                Console.WriteLine("   - {0}", p.ToString());
            }

            Console.WriteLine(" ==== End of List ====");
        }
    }
}

We have new capabilities in this program:

1. We can handle changes to the collection.
2. We can sort the collection.

And what does this all look like:

In conclusion, we have shown the following:

1. Simplest fix to CA1002: change the return type to Collection<T>.
2. Later if you decide to enhance the returned Collection<T> you can do so without affecting prior callers of the method.