Should a List<T> be private?

Question

Hi

I need your opinion on this because I have read a lot of different things on the subject. If you have a List<T> or any kind of list within a class declaration do you make it private and then add or remove items using specific methods or do you make it public?

Your views would be much appreciated with any disadvantages/advantages of each option.

To give an example let's say we have a class Employer with private fields name and List<Employees>. My question is if we should make the employees list private or public and what the advantages/disadvantages be on either case.

Answer 1

If you need to expose a collection to your class' users, you should make a readonly property with a System.Collections.ObjectModel.Collection<T>.

You can then inherit this class and override InsertItem, RemoveItem, and SetItem to run custom logic when the user manipulates the collection.

If you don't want the user to be able to change the collection, you should expose a ReadOnlyCollection<T>.

In your specific example, you should probably expose a ReadOnlyCollection<Employee> with separate mutator methods in Employer.

Answer 2

Depends on the functionality you want. If you just want people to be able to manipulate the list, you could expose it through a read-only property (without the setter). If you want extra code to be executed when users manipulate the list, you should write your own methods, and not expose the list.

Answer 3

for List explicitly yes it should be private depending on what the functionality you're exposing is supposed to do, interfaces such as IEnuemerable, ICollection or IList would be a better choice or if you're exposing a collection See SLaks reply.

Generally exposing internal structure and state is a bad idea and since your object of type List is both, you would want to keep it internal. It might make sense to give the user the ability to iterate over it, to add or remove items to it but you should still keep the List internal and either expose Add/Remove methods or as a minimum expose an interface making it possible to change the type of the internal representation with out affecting the public interface.

Further more if you are exposing using an interface you should go for the narrowst possible interface.

So if the client code only needs to enumerate it. use IEnumerable if client code needs to index use ICollection and so forth.

further if you expose as an IEnumerable you should make sure that what ever you return is in fact read only by either using a read only collection class or by use of an iterator block

EDIT after update In regards to your example. Ask yourself does it make sense that any one except the Employer can change who his employees are? to me that's in the words you've chosen already. The Employer employs the Employee and should have full control over who his/hers employees are. So it that particular case I'd keep it private and expose Hire(IEmployee employee) and Fire(IEMployee employee) that why the code plainly states the intent

Answer 4

As per the refactoring catalog its always better to encasulate the collections. This prevents some one from accidently currupting the data by adding or removing items from the list. If you don't need the functionality of protecting your data from accidental changes you can return a normal list.

By exposing the Add and Remove methods you get the advantage that any changes happens only through these methods.

Answer 5

And if all you want is for someone to be able to enumerate the list, you could expose an iEnumerable whose GetEnumerator function would simply call the list's GetEnumerator function.