DEV Community

André Slupik
André Slupik

Posted on

Seriously, stop using List<T> in APIs

List<T> is to collections what StringBuilder is to strings. It's a mechanism for imperatively building a collection. List<T> is designed for things like

IReadOnlyList<Element> GetElements(IReadOnlyCollection<Element> inputElement) 
{
    var list = new List<Element>();
    foreach(var inputElem in inputElements)
    {
        if (...) 
        {
            list.Add(inputElem);
        }
    }
    return list;
}
Enter fullscreen mode Exit fullscreen mode

Notice that List<T> does not appear in the function signature, our API: it is an implementation detail, a mechanism. What the API says is give me any sort of finite collection, and I'll give you back another finite, indexed collection. The caller does not need to read the implementation to understand this.

Let's see what happens if we replace the input element with List<T>.

IReadOnlyList<Element> GetElements(List<Element> inputElement) 
Enter fullscreen mode Exit fullscreen mode

This is now a confusing and restrictive function signature. It returns a collection, but it can also add, remove or change elements in the input collection. Does it? The caller doesn't know. He'd have to read the code, meaning that our API is no longer a clear abstraction. Furthermore the caller cannot pass in an Array, an ImmutableArray or any other collection, when these would in fact work perfectly fine for our purposes.

Don't take List<T> as an input argument unless you mean to modify that input argument. In that case, your function should probably then return void (or Task, if it's async).

void AddElements(List<T> collection); // this does make sense
Enter fullscreen mode Exit fullscreen mode

Now let's look at the other case, the return type. What would this mean?

List<Element> GetElements(IReadOnlyCollection<Element> inputElement) 
Enter fullscreen mode Exit fullscreen mode

This is not restrictive, but it's equally confusing. The function returns a collection which may then be modified by the caller. Why? Does the function store a reference to it somewhere so it can observe the changes later? Or worse, does it modify it while it's being used? Again, this is just strange API design that prompts me to read the implementation when I shouldn't have to.

I have a hard time coming up with a good reason to return List<T>: if you have an internal mutable collection, surely it's a bad idea to share it with the outside world where it may be modified unexpectedly. Mutable state is best kept local.

Note that strictly speaking, returning IReadOnlyList<T> does not guarantee immutability either, but it does send a strong message in that direction. Strictly speaking, the correct return type would be ImmutableArray<T> or ImmutableList<T>. I'm still on the fence; libraries seem to gravitate towards the IReadOnly interfaces more than System.Collections.Immutable, for whatever reason.

Top comments (0)