16

I am using FxCop and it shows warning for "Don't expose generic list" which suggests use Collection<T> instead of List<T>. The reason why it is preferred, I know all that stuff, as mentioned in this SO post and MSDN and many more articles I have been through.

But my question is this, I am having few methods which does so much heavy calculation and methods accepts parameters of List<T> which is supposed to be faster and good in terms of performance. But FxCop issues warning for this as well as. So one option is that I should declare the parameter as Collection<T>, then use ToList() inside the method and then use it.

So which one is optimized?

"Suppress the warning for this case" OR "use Collection<T> in parameter and then use ToList() inside the method itself".

Mangesh
  • 5,491
  • 5
  • 48
  • 71
Yogesh
  • 3,044
  • 8
  • 33
  • 60
  • 2
    What's wrong with IList? – Jahan Zinedine May 29 '13 at 06:23
  • 1
    IList doesn't give methods like AddRange() & sort – Yogesh May 29 '13 at 06:24
  • 2
    If you need the speed you can use `List<>` as long as you never want the caller to know when the elements of the collection are changed. We have lots of speed-critical calculations (doing analysis of large amounts of ECG data) which collections are far too slow for, so we use `List<>` or even plain arrays in some cases. I guess I'm saying that I think `List<>` is fine for numerics. I'm talking about `List` and `List`, i.e. lists of primitives. – Matthew Watson May 29 '13 at 06:59
  • 1
    Are you sure, that you really will have a performance lag, when using `Collection` instead of `List`? I mean, in your particular case? Also, why just don't use `SuppressMessageAttribute`, if you really know, what are you doing? – Dennis May 29 '13 at 07:17
  • 1
    Asking for the "better" option is going to lead to subjective answers. Can you re-word or clarify so we can provide objective answers? – Paul Turner May 29 '13 at 08:59
  • I use the LinQ Select() which provides ToList(), but there is no ToCollection(). So, I have no choice, but to use List. – Estevez Jan 07 '14 at 12:29

4 Answers4

15

The code analysis/FxCop rules have been written to support framework creators (Microsoft creates a lot of frameworks). A framework is consumed by external parties and you should be careful when you design the public interface. Provided that you are not writing a framework to be consumed by external parties you can simply ignore rules that doesn't provide value to you.

However, one of the reasons that this rule exists is that exposing collections on a class is somewhat difficult. Often the elements in the collection are owned by the containing class and in that case you violate encapsulation if you allow clients to modify the collection used to store the aggregated items. By returning List<T> you allow the clients to modify the collection in many different ways. But often you want to keep track of the items in the collection. E.g. adding a new element might require some additional bookkeeping in the containing class etc. You lose this kind of control when you return a List<T> unless of course you make a copy when you return it (but then the client should understand that they only get a copy of collection and modifications will be ignored).

All in all you can probably improve your class design by avoiding exposing classes like List<T> and being more explicit about how aggregated elements can be added, modified and removed. But if you are in a hurry and just want to crank out some code then using List<T> may be exactly what you need to get the job done.

Martin Liversage
  • 104,481
  • 22
  • 209
  • 256
4

Don't bother using generic lists in public properties as long as you are not coding a framework somebody else want's to extend in the near future. I suggest to suppress the warning. You can refactor your classes later if requirements change.

Andy B.
  • 135
  • 1
  • 9
1

IMHO your interpretation of "Don't expose generic list' which suggests use collection instead of list". Is invalid.

The critical difference between collection and list is that the elements in list are ordered. Some methods may require that passed elements have order. Then we must use in parameter a list.

The key to understand delivered warning is that you should use instead of concrete class List<T> a interface IList<T>.

As the method operate on the list it is not so important what kind of list it is. The key factor is that it is a list.

Concluding the method parameters should be abstract as possible.

Damian Leszczyński - Vash
  • 30,365
  • 9
  • 60
  • 95
0

You should use the type that is most appropriate for your purposes (and suppress the warning if appropriate). If you're passing a bunch of items, and order and uniqueness don't matter, use a collection. If you're passing an ordered collection of items, use a list. If you're passing data such that every item is unique but order doesn't matter, use a set. Use the type that has the semantic meaning appropriate for the exchange. In a few cases where the semantics and the methods that you need don't necessarily align (suppose you need AddRange), make an exception, or use the conversion methods.

user1676075
  • 3,056
  • 1
  • 19
  • 26