Properties of collection type

Question

When the property have some collection type like

public IList<MyItemClass> MyList{ get; }

IMHO, it's better to return empty collection instead of null value.
There are many ways to implement such functionality.

public IList<MyItemClass> MyList{ get; private set; }
public MyClass(){
    MyList = new List<MyItemClass>();
}

This way allow to decrease number of class fields, but you need to put code in each constructor.

private List<MyItemClass> _myList = new List<MyItemClass>();
public IList<MyItemClass> MyList{ get{ return _myList; } }

This is standard way. But when somebody write something to your code he can use private field instead of property and you can get some bugs when you refactor get action.

private List<MyItemClass> _myList;
public IList<MyItemClass> MyList{ get{ return _myList??(_myList = new List<MyItemClass>()); } }

And this is the variant of the previous way with lazy loading.

What are you prefer to return as default value for collection? If this is empty collection how do you implement it?

Answer 1

Option 2 (standard way) and avoid the refactoring issue by ensuring your unit tests check the empty list is returned when you expect.

Answer 2

From the .NET Design Guidelines:

Do provide sensible default values for all properties, ensuring that the defaults do not result in a security hole or an extremely inefficient design.

Extending this principle and combining it with the Principle of Least Surprise should make it abundantly clear that you should always return an empty collection instead of null.

Why put the burden of checking for null on your caller when you can provide a sensible, intuitive default value?

The whole point of encapsulation is to do the work in a single place. It may make that specific class implementation a bit more complex, but it makes using its API simpler.

In most cases I implement the collection as an invariant in the containing type:

public class MyClass
{
    private readonly List<Foo> foos;

    public class MyClass()
    {
        this.foos = new List<Foo>();
    }
}

Notice the use of the readonly keyword which ensures that once set, the list can never be replace or nulled out (but it can still be reset). This protects the list as an invariant of the class, which safes me from writing null checks in the rest of my code.

Lazy loading is also a valid coding idiom, but I only use it when I explicitly need it.

Answer 3

I prefer second variant because it's more obvious and decrease constructor size. But is usually make this fields readonly:

private readonly List<MyItemClass> _myList = new List<MyItemClass>();

Answer 4

Returning null is indeed not ideal. For alternatives; it depends on the use. IMO, this version (copied from the main post) is the simplest yet does everything we need:

private List<MyItemClass> _myList = new List<MyItemClass>();
public IList<MyItemClass> MyList { get { return _myList; } }

and as an advantage, it'll work with XmlSerializer, where-as anything with a private set won't work (a bug; it sees the setter but doesn't notice it can't use it).

The lazy approach is usually overkill IMO, as in most interesting cases you're going to populate the lists anyway.

Answer 5

I usually use the variant where you initialize the list in the constructor. This might make the constructor more "bloated", but the constructors responsibility is constructing the object. Setting up good default values is IMHO completely within its responsibilities. Also, as a bonus you don't have to worry about private fields which makes the code look cleaner.

Sure, refactoring does get a little harder, but not significantly. Don't forget that you can chain your constructors (most of the time anyway) so even if you have several constructors you should be able to only have to write the initializer code once.

Don't forget to document that the default value is an empty collection though. And setting the collection field to readonly is good practice unless you have a good reason to reset it later.