Wednesday, October 27, 2010

Creating properties for list objects? Don't use a setter.

Browsing code today I noticed a potential problem that can cause horrible consequenses if not fixed.

An object exposes a list like so:

public List<Wimwam>Wimwams{get;set;}

which may seem to be quite normal until you look again a the the implications of the property. Here, it is the content of the property being set, namely the collection object itself! This means that it is perfectly possible for a class to fill this list carefully and then have some outside actor change the list under it's nose. This clearly breaks the rules of encapsulation and is a potentially serious bug.

That collection property should in fact be read-only. That is to say that the list itself is not read only but the property to obtain that list should only provide a getter.

The correct way to create the class and the list would be as follows:

public class Doodaa
    private List<Wimwam> _wimwams = new List<Wimwam>();

    public List<Wimwam> Wimwams{get{return _wimwams;}}

In this way, the class encapsulates the list in such a way that the reference to the list object cannot be changed. The contents of the list therefore remain under the ownership of the class.

1 comment:

Bob said...

Just a note to say that the HTML editor for this post has messed up the declaration of the list variable. It should obviously be a list of wimwams.