Is there a need to encapsulate Collections?

Question

say

private List<Item> items;

public Items Items
{
    get { return this.items; }
    set { this.items= value; }
}

or just

public List<Item> Items;

EDIT:

Is there a way to bind to the collection without exposing the public getter?

score 4 · Answer 1 · answered Nov 08 '10 at 14:25

4

I generally expose collections as read-only. There's typically no reason to assign a new collection. I'll instantiate the collection one time in the constructor or property getter. This reduces the possibility of null reference errors.

answered Nov 08 '10 at 14:25

spoulson

21,335
15
77
102

+1, exposing collections to modification is a almost always a bad idea and not what you want. You may want to expose a method to add to the collection (or delete from it), but giving control of the whole collection is generaly not good. – Steve Nov 08 '10 at 14:34
"This reduces the possibility of null reference errors." I think approach is common to all the class members, not specially to collections. – serhio Nov 08 '10 at 14:36
IMO, there is not only "no reason to assign a new collection", it could have side effects. See my answer. – Stefan Steinegger Nov 08 '10 at 14:48

Stefan Steinegger · Answer 2 · 2010-11-08T14:33:09.207

3

Making collections public is quite a bad idea. While encapsulating fields is important in any case, collections are even more important.

The collection itself is usually not an object you actually care about. So the caller of your class shouldn't be able to set it, replace it or even take it away (say: set to null). It should only care about the items in it. If you make it possible to set the list instance from the outside, you risk that several instances of your class share the same list instance. This will have some nasty side effects.

Your class is responsible (at least) to create the list instance to make sure that it is a dedicated list and never a null reference.

Exposing the list by a public getter is actually like fully exposing it. This is the minimal encapsulation you should do, it is actually like no encapsulation at all.

Full encapsulation is done by only exposing the items as IEnumerable and provide Add, Remove, Count and similar operations in separate methods or properties. The more you need, the more it gets complicated. It is useful if you really don't need many basic operations from the outside and manage as much as possible within the class.

edited Nov 08 '10 at 14:33

answered Nov 08 '10 at 14:26

Stefan Steinegger

63,782
15
129
193

sometiles I need to bind to `myObject.Items`, so I need to externalize the collection. – serhio Nov 08 '10 at 14:28
in that case you could still manipulate the collection. Items.Clear(), Items.Add, Items.Remove, etc... – hunter Nov 08 '10 at 14:30
"exposing the items as IEnumerable" - you can always cast it to the initial `List` – serhio Nov 08 '10 at 14:34
1

@serhio: you never can avoid misusing something. IMO, encapsulation it is not about "security", to avoid that anybode does something evil. It is only to make clear how it is intended to be used. You could also use reflection to change the private fields ... but then you know that the class is not intended to be used like this. – Stefan Steinegger Nov 08 '10 at 14:36
@Stefan Steinegger: I am not sure that if exposed as `IEnumerable` that collection remains write-bindable... – serhio Nov 08 '10 at 14:40
@serhio: I doubt. I just said, it should be IEnumerable *IF* you want to hide as much as possible. Not in any case. – Stefan Steinegger Nov 08 '10 at 14:43
There is one situation where it makes sense to have a writable collection property--when the class is intended to encapsulate the *identity* of the collection rather than its contents. For example, a 'viewer' object might provide a means of selecting which object's state should be viewed. Selecting a different object for viewing shouldn't cause anything to be added or removed from any publicly-visible collection; any caches that need to be cleared and rebuilt should be an internal implementation detail. – supercat Nov 28 '12 at 18:07
@supercat: good points. In this case, the collection is not actually owned by the class that references it. – Stefan Steinegger Nov 29 '12 at 08:03
@StefanSteinegger: Precisely. One aspect of Java which .Net borrows, and which I don't like, is that there's nothing declaratively which distinguishes between object references whose purpose is to encapsulate the *mutable state* of the referenced object, its *identity*, both, or neither (presumably encapsulating immutable state only). Mutable objects should generally have one "owner", but that doesn't mean the owner will be the only thing that can mutate them. – supercat Nov 29 '12 at 15:37

score 2 · Answer 3 · edited May 23 '17 at 12:29

2

Public fields are, in general, frowned upon in the .NET world. Also, returning null collections from methods or properties is also frowned upon. Therefore, I'd definitely "encapsulate" the collection.

edited May 23 '17 at 12:29

Community

1
1

answered Nov 08 '10 at 14:29

score 1 · Answer 4 · answered Nov 08 '10 at 14:26

There is a standard in .net that newly instantiated objects have their collections initialized to minimize null references. You will notice most of the framework libraries adhere to this.

public class PotOfUnicorns
{

    private List<Item> items = new List<Item>();

    public Items Items
    {
        get { return this.items; }
        private set { this.items= value; }
    }

}

score 1 · Answer 5 · answered Nov 08 '10 at 14:27

1

Some times I hook it up like so....

List<Item> _items;
public Items Items
{
    get 
    {
        if (_items == null) _items = new List<Items>();
        return _items;
    }
    private set { _items = value; }
}

answered Nov 08 '10 at 14:27

hunter

62,308
19
113
113

score 1 · Answer 6 · answered Nov 08 '10 at 14:34

You are advised against making collection property setters public; see this rule on MSDN. Even better practice is to only expose the interface rather than concrete type.

class Example
{
     public Example()
     {
         this.MyList = new List<string>();
     }

     public IList<string> MyList { get; private set; }
}

Ian Ringrose · Answer 7 · 2010-11-08T22:42:35.483

Creating a custom collection to wrap the standard collection so you can control what your callers do is the best option, however it has costs.

I think it comes down to how many different people will be writing code against your object model and weather these people are part of the same team as you.

You are stopping the callers doing “bad things” by encapsulation your collection; this is likely to save the callers debugging time and also make it easier for your callers to work out how to use your API.

So it is a trade of between your time and the time of the programmers that will be using your object model.

Is there a need to encapsulate Collections?

7 Answers7