Class design - properties default value

Question

Just wondering what the recommended way of tackling this scenario is and why it's better one way or another.

Let's say you have a class that represents a football team, you probably have something similar to this (oversimplified version of course):

public class FootballTeam
{
    public FootballTeam(string name, Person owner)
    {
        Name = name;
        Owner = owner;
    }

    public string Name {get;}

    public Person Trainer {get; set;}

    public Person Owner {get; set;}

    public List<Person> Roster {get; set;}
}

Now, we could argue that even a team with no players still has a roster, only it's an empty roster (as oppose to a non-existing roster).

So it would maybe make more sense to declare the Roster like this:

public List<Person> Roster {get; set;} = new List<Person>();

But this still suffers from a problem in my opinion:

Because Roster still has a setter, it does nothing to prevent it from being set to null, which also makes it easy to wrongly assume that it can never be null.

So instead Roster should look like this:

public List<Person> Roster {get;} = new List<Person>();

So now we're semantically correct in the sense that a team always has a roster, only sometimes the roster contains no players. And we simplify things for the caller in the sense that they don't have to worry about null references.

But in doing so we lost the ability to easily replace the complete existing roster with another one we already had, we're bound to that instance which we have to add/remove/clear Persons from, if we have two teams with the same players checking their Rosters for equality will return false, etc.

So a couple of questions:

1. What would be the best way to tackle this? Hiding the List<Person> and presenting forwarding methods to indirectly interact with it seems overkill and of little added bonus. Removing the setter removes some simplicity and functionality, allowing the Roster to be null seems even worse to me.

2. Assuming that the answer to 1. involves assigning a default value to the Roster property, are there any best practices as to where to do this? i.e., directly at the property like on the examples above or on the constructor?

3. You could also argue that a team must always have an Owner as well, although it does not necessarily need to always have a Trainer. How would you handle this as well? Forwarding methods or checking if value == null on the setter and if so throw?

Answer 1

If team "owns" the list, I would expect to see something like this:

public class FootballTeam
{
    private readonly List<Person> _roster = new List<Person>();
    public FootballTeam(string name, Person owner)
    {
        Name = name;
        Owner = owner;
    }

    public string Name {get;}

    public Person Trainer {get; set;}

    public Person Owner {get; set;}

    public IEnumerable<Person> Roster => _roster.AsReadOnly();

    public void AddPlayer(Person player) {
       _roster.Add(player);
       //Other logic
    }

    public void RemovePlayer(Person player) {
       //What if the player isn't on our roster?
       //Other logic?
       _roster.Remove(player);
    }

    public void ReplaceRoster(IEnumerable<Person> players) {
       _roster.Clear();
       _roster.AddRange(players);
    }
}

I wouldn't normally have added the ReplaceRoster method but apparently that's one of your use cases. Note the fact that we're using a List is now encapsulated and we've moved up a semantic level in the methods that our class supports.

Other methods within our class can rely on _roster being non-null. Consumers of our class can always rely on getting the IEnumerable<Person> even when the roster is empty.

If the team doesn't "own" the list, I wouldn't expect it to have the property at all.

Answer 2

Try this:

private List<Person> _roster = new List<Person>();
public List<Person> Roster {get=>_roster; set=>_roster = value ?? new List<Person>();}

This will prevent from assigning null to _roster.