In API code, is it a good idea to use a private setter for list type C# properties?
I'm looking for an opinion on something. Consider the following code from a class called SomeApiObject
:
// Property with private setter
public IList<string> SomeList
{
get;
private set;
}
// Constructor
public SomeApiObject()
{
SomeList = new List<string>();
}
With this setup, users of the class SomeApiObject
cannot reassign the SomeList
property, but what they can do is to manipulate the existing list by using methods such as Add()
, Remove()
, and Clear()
.
The upside of this pattern is that the property is guaranteed to never be null
, which can be a very convenient assumption to make as a user is working with the API, since it means the user can always iterate over the list, get the list's size, or add to it without ever having to check for null
.
I see a few downsides. For one, it's not necessarily obvious to a user that the list is intended to be writable by manipulating its contents. For another, I could envision situations where manipulating the list is less convenient syntactically or possibly worse in performance than assigning a new list.
I'm on the fence with this one, and I'm seeking opinions.
- Are the potential downsides just too obnoxious for a user of the API?
- Is the guarantee of never being
null
as nice a feature as I think it is?
EDIT:
I'm already convinced of the benefits of using some sort of "never null" pattern. What I'm more interested in is for someone to play devil's advocate and show me why having a private setter on a list that's meant to be manipulated might be annoying and/or prohibitive from the perspective of a user of the API.
I released a .NET API wrapper some time ago, and so far a few users have expressed con开发者_如何学Gofusion over how to assign values to properties like this one.
Never null is a good design feature. In regards to only exposing lists as read only properties this is put forward as a recommendation in the my favorite guidelines book: http://www.amazon.com/Framework-Design-Guidelines-Conventions-Libraries/dp/0321545613
Though a better approach is not to allow external callers to manipulate the state of your object directly and to make the class immutable:
public class MyClass
{
private List<string> _inner = new List<string>();
public IEnumerable<string> Items
{
get { return _inner.GetEnumerator(); }
}
public void AddItem(string item);
{
_inner.Add(item);
}
}
If you want to remove the possibility of users getting a reference to the list then simply manipulating it externally in an unintended manner, then you should change your property to only return an IEnumerable
instead.
public IEnumerable<string> SomeList
{
get { return list.GetEnumerator(); }
private set {}
}
This will allow the user to still make use of the collection (also will support linq) and will protect the collection from external manipulation.
One other point to consider is that the question depends on whether the object "owns" the list (composition) or "has" the list (aggregation). If the object owns the list, the setter should be private; if it simply has the list from another object, the setter could possibly be public.
A complication in the composition case (other than the possibility of the list being assigned a null value) is that a public setter causes the class to cede all control over the implementation of the list. Suppose, for instance, you have the implementation:
public IList<string> SomeList
{
get;
private set;
}
public SomeApiObject()
{
SomeList = new List<string>();
}
Now suppose in a subsequent version, you want to use SomeSpecializedList
, which implements IList<string>
, instead of List<String>
. You could easily refactor:
private SomeSpecializedList specializedList;
public IList<string> SomeList
{
get {return specializedList;}
private set {specializedList = value as SomeSpecializedList;}
}
public SomeApiObject()
{
SomeList = new SomeSpecializedList<string>();
}
Only the private implementation has changed; users of the class are unaffected. But if the setter were public, the client could have potentially passed any IList instance into SomeList, and this would be a breaking change.
it depends on the situation. in some cases, it might make sense to be able to have a null list, in other not so much. you have to ask yourself if it makes sense. .net itself does it both ways.
As much as I hate saying this, it really depends on what you're trying to do with your API. If you want to provide the user with a collection that they can work with, then @Matthew's answer is appropriate. If you want to hide the collection or only allow certain actions to be permitted (like Add
, Remove
, etc.), then you could hide the collection and expose a facade to the collection:
private IList<string> _someList = new List<string>();
public void Add(string item){ _someList.Add(item); }
public string Remove(string item) { return _someList.Remove(item); }
...
Hope this helps.
精彩评论