63

According to FXCop, List should not be exposed in an API object model. Why is this considered bad practice?

David Robbins
  • 9,996
  • 7
  • 51
  • 82

6 Answers6

56

I agree with moose-in-the-jungle here: List<T> is an unconstrained, bloated object that has a lot of "baggage" in it.

Fortunately the solution is simple: expose IList<T> instead.

It exposes a barebones interface that has most all of List<T>'s methods (with the exception of things like AddRange()) and it doesn't constrain you to the specific List<T> type, which allows your API consumers to use their own custom implementers of IList<T>.

For even more flexibility, consider exposing some collections to IEnumerable<T>, when appropriate.

Jon Limjap
  • 94,284
  • 15
  • 101
  • 152
  • what would you recommend for a web service (where a method can't return/take an interface)? – kpollock Dec 23 '08 at 12:50
  • 1
    kpollock -- unfortunately SOAP web services *will* turn any IList into an array of type T[] --> serialization doesn't support the generic list type. – Jon Limjap Dec 30 '08 at 13:33
  • 8
    Just because you use IList, doesn't mean that you save any memory if that's what you're getting at. You instantiate it with new List() and the IList variable still points to a List in memory. Other than that, I think its usually beneficial to encapsulate a collection instead of directly exposing to client code. – Jeff LaFay Jun 08 '11 at 18:31
  • Of course @jlafay but, that wasn't the point of the answer -- the point is simply that there are other implementers of IList or IEnumerable that *can* be pointed to the variable, and I think that's the point of FxCop as well. On the other hand, FxCop rules can be modified to ignore this particular rule ;) – Jon Limjap Jun 09 '11 at 08:47
  • 4
    Be careful with `IList` though. [`Array` also implements `IList`](http://stackoverflow.com/q/5968708/210336) and will throw an exception on `Add` and `Remove`. This can be confusing as a lot of people won't expect this. – Matthijs Wessels Mar 28 '14 at 12:47
  • @MatthijsWessels agreed, Array is a big problem for IList - see my answer here https://stackoverflow.com/a/36570307/1154384 – perfectionist Oct 05 '17 at 10:19
28

There are the 2 main reasons:

  • List<T> is a rather bloated type with many members not relevant in many scenarios (is too “busy” for public object models).
  • The class is unsealed, but not specifically designed to be extended (you cannot override any members)
e11s
  • 4,123
  • 3
  • 29
  • 28
  • 7
    Exactly right. Here's a link to Krzysztof Cwalina on the subject: http://blogs.msdn.com/kcwalina/archive/2005/09/26/474010.aspx – Ryan Lundy Dec 23 '08 at 02:19
6

It's only considered bad practice if you are writing an API that will be used by thousands or millions of developers.

The .NET framework design guidelines are meant for Microsoft's public APIs.

If you have an API that's not being used by a lot of people, you should ignore the warning.

Scott Wisniewski
  • 24,561
  • 8
  • 60
  • 89
  • 3
    While it's true that some of the FxCop rules are framework design guidelines and may not be appropriate for some projects, even libraries designed for small teams can benefit from following them. Have you ever told 5 developers that they'll have to change their projects because you need to make a breaking change to a library (say to observe changes as @Kaagle mentioned)? Have developers on your team ever done something you didn't expect, like modifying the contents of a list property that you intended to be read-only? – TrueWill Jan 07 '12 at 16:41
  • 2
    If a guideline doesn't make sense in your situation, disable it in the FxCop project - but review the [guideline reasons](http://www.amazon.com/Framework-Design-Guidelines-Conventions-Libraries/dp/0321545613) carefully first. – TrueWill Jan 07 '12 at 16:42
3

i think you dont want your consumers adding new elements into your return. An API should be clear and complete and if it returns an array, it should return the exact data structure. I dont think it has anything to do with T per say but rather returning a List<> instead of an array [] directly

leora
  • 188,729
  • 360
  • 878
  • 1,366
  • 4
    -1. In particular, [Properties should not return arrays](http://msdn.microsoft.com/en-us/library/0fss9skc%28v=VS.100%29.aspx). While methods may, in general collections, enumerable sequences, and interfaces are more idiomatic to C#. – TrueWill Jan 07 '12 at 16:26
3

One reason is because List isn't something you can simulate. Even in less-popular libraries, I've seen iterations that used to expose a List object as an IList due to this recommendation, and in later versions decided to not store the data in a list at all (perhaps in a database). Because it was an IList, it wasn't a breaking change to change the implementation underneath the clients and yet keep everyone working.

Andrew Arnott
  • 80,040
  • 26
  • 132
  • 171
2

One of the reason is that user will be able to change the list and owner of the list will not know about this, while in some cases it must do some stuff after adding/removing items to/from the list. Even if it isn't required now it can become a requirement in future. So it is better to add AddXXX / RemoveXXX method to the owner of the class and expose list an an IEnumerable or (which is better in my opinion) expose it as an IList and use ObservableCollection from WindowsBase.

Anton Kolesov
  • 7,014
  • 1
  • 17
  • 5
  • 1
    Exposing `List` as `IList` or `IEnumerable` doesn't protect against the user modifying the list (after a cast in the case of `IEnumerable`). If you want to return an immutable list, return `ReadOnlyCollection` or use `IEnumerable.AsReadOnly()`, which can be returned as `IList`. – Joe May 24 '13 at 07:41