Is AsList() better than ToList() with IDbConnection.Query() which returns IEnumerable?

Question

I read this answer from Marc Gravell (@MarcGravell): https://mcmap.net/q/719678/-dapper-vs-ado-net-with-reflection-which-is-faster

The last line says:

As a minor optimization to your code: prefer AsList() to ToList() to avoid creating a copy.

That statement is about QueryMultiple() which returns GridReader.

In my understanding, System.Linq provides an extension method IEnumerable.ToList(). Following is from Microsoft about ToList().

The ToList(IEnumerable) method forces immediate query evaluation and returns a List that contains the query results. You can append this method to your query in order to obtain a cached copy of the query results.

IDbConnection.Query() will ALWAYS return IEnumerable or null. Null-check could be easily done in calling code. What difference does AsList makes then?

If my understanding is correct, AsList will always internally call ToList which will create a copy.

Considering this, is AsList() better than ToList() with IDbConnection.Query() which returns IEnumerable? If yes; why?

What is that AsList() does internally that makes it a better choice in this case?

Unfeeling

Look at the source and you will see comments that indicate that if the data is already in a List data structure, then that list (i.e. instance) will be returned, rather than creating a new (i.e. copy) List. – Lituus

@KennethK.: Agreed. But IDbConnection.Query() will ALWAYS return IEnumerable or null. Null check could be easily done in calling code. What difference does AsList makes then? – Unfeeling

Answer 1

AsList is a custom Dapper extension method. All it does is checks if IEnumerable<T> you pass to it is really List<T>. If it is - it returns it back, just casts to List<T>. If it's not - it calls regular ToList. The point is - ToList() always creates a copy, even if what you pass to it is already a list. AsList() method avoids doing this copy, and so is useful if such copy is unnecessary.

In this specific scenario, you have the following code:

multipleresult.Read<MerchantProduct>()

where multipleresult is GridReader. Read has buffered argument which is true by default. When its true - Read will really return List<T>, so by calling ToList you will copy that list again without much reason.

The same is true for IDbConnection.Query() - is also has buffered parameter, which is true by default, so it will also by default return List<T>.

If you prefer to use ToList() - you can pass buffered: false to Query() or Read() to avoid creating that additional copy.

Vert

The extension can hurt imo. If someone changes the internal implementation, for example from list to array, the AsList suddenly returns a new list instead of returning the same instance. That might break code silently. – Opine

@TimSchmelter that's theoretically valid point, though I don't see how it can break code in this case (for dapper queries). You don't really care if it will return the same list or "new" one in this case - they are both "new" for you. – Vert

@TimSchmelter if I change the internal implementation of Dapper, I'll be sure to do something like annotate AsList() with [Obsolete("...", false)] with a message explaining what and why; I doubt it will change, however – Tribesman

@MarcGravell and why introduce that buffered parameter at all, and even set by default? To prevent accidental multiple executions of query by users unaware of how IEnumerable works? – Vert

@Vert partly that, although actually a more common scenario would be reading it once but deferring it - at which point the connection could be closed or another reader could be active; also partly to provide a unified API when you really do want raw streaming access (buffered: false) – Tribesman

Answer 2

This extension is a custom dapper extension which does an additional check before calling ToList. Source:

public static List<T> AsList<T>(this IEnumerable<T> source) 
    => (source == null || source is List<T>) ? (List<T>)source : source.ToList();

• ToList always creates a new List<T> instance and fills it with the given items
• AsList checks if the sequence is already a List<T>, then it will just cast it

Of course this approach can be more efficient because casting something is much less work than creating and filling something new. So it's completely different.

This is kind of opinion based, but i find this dangerous. Someone might overlook the AsList and reads ToList or just don't know the difference. It's dangerous if someone changes the code later.

So for example a method that takes IEnumerable<T> which uses AsList:

public static List<T> GetResult<T>(IEnumerable<T> seq)
{
    if(some condition here)
    {
        seq = seq.Where(some predicate here);
    }
    return seq.AsList()
}

Now code called this method with a list:

IEnumerable<string> sequence = (gets a list from somewhere)
List<string> userList = GetResult(sequence);

Later someone decides that an array is more appropriate here:

IEnumerable<string> sequence = (gets an array from somewhere)
List<string> userList = GetResult(sequence);

This doesn't really hurt until now. Now a new List is initialized and filled because the source is not a list and can't be casted. So it's just less efficient. But if the logic also relied on the list being the same reference, this won't work anymore.

if(userList == seq)
{
    // do something
}

This is always false once the array is used . So the code was broken silently.

To cut a long story short: i don't like the AsList method. You can always check the type yourself.

Opine

The key point here is that AsList() is intended for use with Dapper to avoid a minor alloc; it is not intended for all purposes, although frankly it works fine in many of them – Tribesman

@MarcGravell: Well, you can use this extension also outside if you add using Dapper;, it's a public extension method of IEnumerable<T>. I tried to answer dapper independently. People might find it very useful so they adopt it in their own extension library. I just wanted to mention the drawback – Opine